[SOT][3.12] replace `POP_JUMP_{BACKWARD,FORWARD}_IF_{TRUE,FALSE}` to `POP_JUMP_IF_{TRUE,FALSE}` #62155

gouzil · 2024-02-27T13:02:48Z

PR types

Others

PR changes

Others

Description

修复

POP_JUMP_BACKWARD_IF_TRUE -> POP_JUMP_IF_TRUE
POP_JUMP_BACKWARD_IF_FALSE -> POP_JUMP_IF_FALSE
POP_JUMP_FORWARD_IF_TRUE -> POP_JUMP_IF_TRUE
POP_JUMP_FORWARD_IF_FALSE -> POP_JUMP_IF_FALSE
修复 for 打断在FOR_ITER后没能正确生成END_FOR

预拦截

对FOR_ITER跳转的拦截check_for_iter_jump_to

参考链接:

相关链接:

# Conflicts: # python/paddle/jit/sot/opcode_translator/executor/opcode_executor.py # python/paddle/jit/sot/opcode_translator/executor/pycode_generator.py

# Conflicts: # python/paddle/jit/sot/opcode_translator/executor/opcode_executor.py

paddle-bot · 2024-02-27T13:02:53Z

你的PR提交成功，感谢你对开源项目的贡献!
请关注后续CI自动化测试结果，详情请参考Paddle-CI手册。
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

diadestiny · 2024-02-28T15:49:04Z

python/paddle/jit/sot/opcode_translator/executor/opcode_executor.py

@@ -2194,6 +2216,8 @@ def create_inline_call_fn():
            )

            nop_for_break = pycode_gen.add_instr("NOP")
+            if sys.version_info >= (3, 12):


我在测test_side_effects.py单测的时候，发现在3.12resume的FOR_ITER字节码会出现结束循环体的位置会比上一次情况往后多一步，导致这个单测出错；感觉和这里的END_FOR的生成有关，但是不理解为啥3.12要专门生成END_FOR，想请问下

是的这个 PR 就是来解决这个问题的，具体表现情况为下面三种情况:

生成跳转错误

[Resumed Function]: Inline call for loop function $resume_1@for_list_1_af1a0 39 0 RESUME 0 2 LOAD_FAST 2 (object_168) >> 4 FOR_ITER 24 (to 56) ... 54 END_FOR >> 56 LOAD_FAST 1 (i) 58 LOAD_FAST 0 (x) 60 LOAD_FAST 2 (object_168) 62 BUILD_TUPLE 3 64 RETURN_VALUE

END_FOR字节码不生成

[Resumed Function]: Inline call for loop function $resume_1@undefined_var_case_0_af1a0 269 0 RESUME 0 2 LOAD_FAST 2 (object_165) >> 4 FOR_ITER 38 (84) ... 74 JUMP_FORWARD 1 (to 78) 76 JUMP_FORWARD 2 (to 82) >> 78 NOP 80 JUMP_BACKWARD 39 (to 4) >> 82 NOP 86 LOAD_FAST 0 (i) ...

resume错误

在 Python3.12 下

13 0 RESUME 0 2 END_FOR # <-- 这里 resume 错误 4 LOAD_FAST_CHECK 0 (zzz) 6 LOAD_CONST 2 (1) 8 BINARY_OP 0 (+) 10 RETURN_VALUE

在 Python3.11 下

13 0 RESUME 0 2 LOAD_FAST 0 (zzz) 4 LOAD_CONST 2 (1) 6 BINARY_OP 0 (+) 14 10 STORE_FAST 0 (zzz) 12 LOAD_FAST 0 (zzz) 14 RETURN_VALUE

在 python3.12 _inline_call_for_loop理论上正常生成的字节码为

39 0 RESUME 0 2 LOAD_FAST 2 (object_168) >> 4 FOR_ITER 27 (to 60) 41 8 STORE_FAST 1 (i) 10 LOAD_FAST 0 (x) 12 LOAD_FAST 1 (i) 14 BINARY_OP 13 (+=) 43 18 STORE_FAST 0 (x) 20 LOAD_FAST 0 (x) 22 LOAD_CONST 2 (2) 24 COMPARE_OP 68 (>) 44 28 POP_JUMP_IF_FALSE 6 (to 42) 30 LOAD_FAST 0 (x) 32 LOAD_CONST 3 (1) 34 BINARY_OP 13 (+=) 38 STORE_FAST 0 (x) 46 40 JUMP_FORWARD 7 (to 56) >> 42 LOAD_FAST 0 (x) 44 LOAD_CONST 3 (1) 46 BINARY_OP 23 (-=) 50 STORE_FAST 0 (x) 52 JUMP_FORWARD 1 (to 56) 54 JUMP_FORWARD 2 (to 62) # 无用 >> 56 NOP 58 JUMP_BACKWARD 28 (to 4) >> 60 END_FOR >> 62 NOP 64 LOAD_FAST 1 (i) 66 LOAD_FAST 0 (x) 68 LOAD_FAST 2 (object_168) 70 BUILD_TUPLE 3 72 RETURN_VALUE

在 python3.11 _inline_call_for_loop生成的字节码为

39 0 RESUME 0 2 LOAD_FAST 2 (object_168) >> 4 FOR_ITER 27 (to 60) 41 6 STORE_FAST 1 (i) 8 LOAD_FAST 0 (x) 10 LOAD_FAST 1 (i) 12 BINARY_OP 13 (+=) 43 16 STORE_FAST 0 (x) 18 LOAD_FAST 0 (x) 20 LOAD_CONST 2 (2) 22 COMPARE_OP 4 (>) 44 28 POP_JUMP_FORWARD_IF_FALSE 6 (to 42) 30 LOAD_FAST 0 (x) 32 LOAD_CONST 3 (1) 34 BINARY_OP 13 (+=) 38 STORE_FAST 0 (x) 46 40 JUMP_FORWARD 7 (to 56) >> 42 LOAD_FAST 0 (x) 44 LOAD_CONST 3 (1) 46 BINARY_OP 23 (-=) 50 STORE_FAST 0 (x) 52 JUMP_FORWARD 1 (to 56) 54 JUMP_FORWARD 2 (to 60) >> 56 NOP 58 JUMP_BACKWARD 28 (to 4) >> 60 NOP 62 LOAD_FAST 1 (i) 64 LOAD_FAST 0 (x) 66 LOAD_FAST 2 (object_168) 68 BUILD_TUPLE 3 70 RETURN_VALUE

我们可以看到FOR_ITER在 3.11 下会让 iterator 结束后跳出整个循环体内的字节码（也就是迭代器耗尽时），而在 3.12 下整个 for 是由 FOR_ITER 和 END_FOR 组成的，在大部分FOR_ITER及其超指令会在 FOR_ITER 字节码内跳过 END_FOR 字节码运行（仅做标识）。

下面的内容可能说的不对：

关于为什么需要专门生成END_FOR(或许也可以直接拷贝):

因为需要与FOR_ITER对应。

# 2.2. copy main logic pycode_gen.extend_instrs(origin_instrs[start_idx:end_idx+1])

关于生成跳转错误

这个我目前是定位到 code gen 的 assemble 只生成了 opcode和arg而没有其他属性(就是argval这些信息)，当然也可能是我们在生成的问题

SigureMo · 2024-03-03T06:06:00Z

python/paddle/jit/sot/opcode_translator/executor/opcode_executor.py

@@ -640,6 +640,10 @@ def _rot_top_n(self, n: int):
    def POP_TOP(self, instr: Instruction):
        self.stack.pop()

+    def END_FOR(self, instr: Instruction):


什么时候会执行到 END_FOR 呢？

这里跑不到，已删除

SigureMo · 2024-03-03T06:06:54Z

python/paddle/jit/sot/opcode_translator/executor/opcode_executor.py

+            # skip resume END_FOR in python3.12
+            if (
+                sys.version_info >= (3, 12)
+                and origin_instrs[loop_body_end_idx].opname == "END_FOR"


需要 and 么？有可能不是 END_FOR 么？是否可以 assert 而不是作为条件呢？

SigureMo · 2024-03-03T06:08:14Z

python/paddle/jit/sot/opcode_translator/executor/opcode_executor.py

+
+        if sys.version_info >= (3, 12):
+            end_for = self._graph.pycode_gen.add_instr("END_FOR")
+
        nop = self._graph.pycode_gen.add_instr("NOP")


3.12 能否直接用 END_FOR 呢，就不需要先插一个 NOP 再插一个 END_FOR 了，这里的 NOP 应该是冗余的

变量名直接统一用 end_for 就好，也不需要两个版本一个用 end_for 一个用 nop 了

POP_JUMP_IF_FALSE并不能跳到END_FOR啊

>> 90 FOR_ITER 19 (to 132) 94 STORE_FAST 0 (i) 96 LOAD_GLOBAL 11 (NULL + $resume_90@undefined_var_case_1_dc807) 106 LOAD_FAST 0 (i) 108 LOAD_FAST 1 (aaa) 110 LOAD_CONST 5 (True) 112 CALL 3 120 UNPACK_SEQUENCE 3 124 STORE_FAST 1 (aaa) 126 STORE_FAST 0 (i) 128 POP_JUMP_IF_FALSE 2 (to 134) 130 JUMP_BACKWARD 21 (to 90) >> 132 END_FOR >> 134 NOP

SigureMo · 2024-03-03T06:11:50Z

python/paddle/jit/sot/opcode_translator/executor/opcode_inline_executor.py

@@ -316,6 +316,8 @@ def FOR_ITER(self, instr: Instruction):
                self.stack.pop()
                assert isinstance(instr.jump_to, Instruction)
                self._lasti = self.indexof(instr.jump_to)
+                next_instr = self._instructions[self._lasti]
+                self._lasti += int(next_instr.opname == 'END_FOR')


同样，3.12 下是否可以 assert 而不是判断，以免生成代码出现问题

OpcodeExecutor.FOR_ITER 可以同样这样考虑下

SigureMo · 2024-03-03T06:14:45Z

python/paddle/jit/sot/opcode_translator/instruction_utils/instruction_utils.py

+                assert instrs.jump_to is not None
+                assert instrs.arg is not None
+                assert instrs.offset is not None
+                instrs.arg -= 1


SigureMo · 2024-03-03T13:26:28Z

python/paddle/jit/sot/opcode_translator/instruction_utils/instruction_utils.py

@@ -239,6 +245,9 @@ def relocate_jump_target(instructions: list[Instruction]) -> None:
            if instr.opname in ABS_JUMP:
                new_arg = jump_target
            else:  # instr.opname in REL_JUMP
+                if sys.version_info >= (3, 12):
+                    cache = PYOPCODE_CACHE_SIZE.get(instr.opname, 0)
+                    jump_target -= 2 * cache
                new_arg = jump_target - instr.offset - 2


cache_size = PYOPCODE_CACHE_SIZE.get(instr.opname, 0) new_arg = jump_target - 2 * (cache_size + instr.offset + 1)

这样就不用 if 了吧，而且以后版本也通用

SigureMo · 2024-03-03T13:31:59Z

python/paddle/jit/sot/opcode_translator/instruction_utils/instruction_pass.py

    """
    find out the local var names which is only stored once
    """
    loaded_vars = {}
    for instr in instrs:
-        if instr.opname == "LOAD_FAST":
+        if instr.opname == "LOAD_FAST" or instr.opname == "LOAD_FAST_CHECK":


Suggested change

if instr.opname == "LOAD_FAST" or instr.opname == "LOAD_FAST_CHECK":

if instr.opname in ["LOAD_FAST", "LOAD_FAST_CHECK"]:

SigureMo · 2024-03-03T13:32:24Z

python/paddle/jit/sot/opcode_translator/instruction_utils/instruction_pass.py

    """
    stack = []
    opcode_pairs = []
    for instr in instrs:
-        if instr.opname == "LOAD_FAST":
+        if instr.opname == "LOAD_FAST" or instr.opname == "LOAD_FAST_CHECK":


Suggested change

if instr.opname == "LOAD_FAST" or instr.opname == "LOAD_FAST_CHECK":

if instr.opname in ["LOAD_FAST", "LOAD_FAST_CHECK"]:

SigureMo · 2024-03-03T13:33:39Z

python/paddle/jit/sot/opcode_translator/instruction_utils/instruction_utils.py

+        if (
+            instrs.opname == 'LOAD_FAST'
+            or instrs.opname == 'LOAD_FAST_CHECK'
+            or instrs.opname == 'STORE_FAST'
+        ):


Suggested change

if (

instrs.opname == 'LOAD_FAST'

or instrs.opname == 'LOAD_FAST_CHECK'

or instrs.opname == 'STORE_FAST'

):

if instrs.opname in ['LOAD_FAST', 'LOAD_FAST_CHECK', 'STORE_FAST']:

SigureMo

LGTMeow

gouzil added 7 commits February 26, 2024 16:06

debug END_FOR

ef4f58d

Merge branch 'develop' of github.com:gouzil/Paddle into sot_fix_END_FOR

ec6c2aa

# Conflicts: # python/paddle/jit/sot/opcode_translator/executor/opcode_executor.py # python/paddle/jit/sot/opcode_translator/executor/pycode_generator.py

fix

601a67a

tmp storage

c4e53a3

Merge branch 'develop' of github.com:gouzil/Paddle into sot_fix_END_FOR

234ef7a

# Conflicts: # python/paddle/jit/sot/opcode_translator/executor/opcode_executor.py

Merge branch 'develop' of github.com:gouzil/Paddle into sot_fix_END_FOR

123abe8

supplement LOAD_FAST_CHECK

b1e1477

paddle-bot bot added the contributor External developers label Feb 27, 2024

gouzil changed the title ~~[SOT][3.12] replace POP_JUMP_{BACKWARD,FORWARD}_IF_{TRUE,FALSE} to POP_JUMP_IF_{TRUE,FALSE} and add LOAD_FAST_CHECK opcode~~ [SOT][3.12] replace POP_JUMP_{BACKWARD,FORWARD}_IF_{TRUE,FALSE} to POP_JUMP_IF_{TRUE,FALSE} and add LOAD_FAST_CHECK OpCode Feb 27, 2024

diadestiny reviewed Feb 28, 2024

View reviewed changes

SigureMo self-assigned this Feb 28, 2024

gouzil added 3 commits March 2, 2024 10:08

Merge branch 'develop' of github.com:gouzil/Paddle into sot_fix_END_FOR

5a4b1fa

fix args and fix FOR_ITER jump_to error

cd1ec5c

add check

fc254de

gouzil changed the title ~~[SOT][3.12] replace POP_JUMP_{BACKWARD,FORWARD}_IF_{TRUE,FALSE} to POP_JUMP_IF_{TRUE,FALSE} and add LOAD_FAST_CHECK OpCode~~ [SOT][3.12] replace POP_JUMP_{BACKWARD,FORWARD}_IF_{TRUE,FALSE} to POP_JUMP_IF_{TRUE,FALSE} Mar 2, 2024

gouzil mentioned this pull request Mar 2, 2024

[Don't merge] debug END_FOR #62261

Closed

gouzil requested a review from SigureMo March 3, 2024 06:02

SigureMo reviewed Mar 3, 2024

View reviewed changes

fix relocate_jump_target and add END_FOR check

29d3960

SigureMo reviewed Mar 3, 2024

View reviewed changes

simplify if

e40f30e

SigureMo approved these changes Mar 3, 2024

View reviewed changes

SigureMo mentioned this pull request Mar 4, 2024

SOT Python3.12 单测支持情况 #61174

Closed

SigureMo merged commit de1777b into PaddlePaddle:develop Mar 4, 2024
30 checks passed

gouzil mentioned this pull request Mar 6, 2024

[WeeklyReports] 2024.02.25~2024.03.08 周报汇总 PFCCLab/Camp#132

Closed

28 tasks

gouzil deleted the sot_fix_END_FOR branch April 23, 2024 11:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SOT][3.12] replace `POP_JUMP_{BACKWARD,FORWARD}_IF_{TRUE,FALSE}` to `POP_JUMP_IF_{TRUE,FALSE}` #62155

[SOT][3.12] replace `POP_JUMP_{BACKWARD,FORWARD}_IF_{TRUE,FALSE}` to `POP_JUMP_IF_{TRUE,FALSE}` #62155

gouzil commented Feb 27, 2024 •

edited

Loading

paddle-bot bot commented Feb 27, 2024

diadestiny Feb 28, 2024

gouzil Feb 29, 2024 •

edited

Loading

SigureMo Mar 3, 2024

gouzil Mar 3, 2024

SigureMo Mar 3, 2024

gouzil Mar 3, 2024

SigureMo Mar 3, 2024

gouzil Mar 3, 2024

SigureMo Mar 3, 2024

gouzil Mar 3, 2024

SigureMo Mar 3, 2024

SigureMo Mar 3, 2024

SigureMo Mar 3, 2024

SigureMo Mar 3, 2024

SigureMo Mar 3, 2024

SigureMo left a comment

	if instr.opname == "LOAD_FAST" or instr.opname == "LOAD_FAST_CHECK":
	if instr.opname in ["LOAD_FAST", "LOAD_FAST_CHECK"]:

[SOT][3.12] replace POP_JUMP_{BACKWARD,FORWARD}_IF_{TRUE,FALSE} to POP_JUMP_IF_{TRUE,FALSE} #62155

[SOT][3.12] replace POP_JUMP_{BACKWARD,FORWARD}_IF_{TRUE,FALSE} to POP_JUMP_IF_{TRUE,FALSE} #62155

Conversation

gouzil commented Feb 27, 2024 • edited Loading

PR types

PR changes

Description

paddle-bot bot commented Feb 27, 2024

Choose a reason for hiding this comment

gouzil Feb 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SigureMo left a comment

Choose a reason for hiding this comment

[SOT][3.12] replace `POP_JUMP_{BACKWARD,FORWARD}_IF_{TRUE,FALSE}` to `POP_JUMP_IF_{TRUE,FALSE}` #62155

[SOT][3.12] replace `POP_JUMP_{BACKWARD,FORWARD}_IF_{TRUE,FALSE}` to `POP_JUMP_IF_{TRUE,FALSE}` #62155

gouzil commented Feb 27, 2024 •

edited

Loading

gouzil Feb 29, 2024 •

edited

Loading