Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SOT][3.12] Support CALL_INTRINSIC_1 opcode in Python 3.12 #61995

Merged
merged 8 commits into from
Feb 26, 2024

Conversation

gouzil
Copy link
Member

@gouzil gouzil commented Feb 22, 2024

PR types

Others

PR changes

Others

Description

python 3.12 支持 CALL_INTRINSIC_1

参考链接:

相关链接:

(TODO: 好像跑不到BreakGraphError让我测测)

Copy link

paddle-bot bot commented Feb 22, 2024

你的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.

@paddle-bot paddle-bot bot added the contributor External developers label Feb 22, 2024
Comment on lines 1504 to 1528
class Intrinsics_UnaryFunctions(Enum):
INTRINSIC_1_INVALID = 0
INTRINSIC_PRINT = 1 # no support, print
INTRINSIC_IMPORT_STAR = 2 # no support, `from module import *`
INTRINSIC_STOPITERATION_ERROR = (
3 # no support, generator or coroutine
)
INTRINSIC_ASYNC_GEN_WRAP = 4 # no support, async
INTRINSIC_UNARY_POSITIVE = 5
INTRINSIC_LIST_TO_TUPLE = 6
INTRINSIC_TYPEVAR = 7 # no support, PEP 695
INTRINSIC_PARAMSPEC = 8 # no support, PEP 695
INTRINSIC_TYPEVARTUPLE = 9 # no support, PEP 695
INTRINSIC_SUBSCRIPT_GENERIC = 10 # no support, PEP 695
INTRINSIC_TYPEALIAS = 11 # no support, PEP 695

def to_func(self):
if self == self.INTRINSIC_1_INVALID:
raise RuntimeError("invalid intrinsic function")
elif self == self.INTRINSIC_UNARY_POSITIVE:
return OpcodeExecutor_.UNARY_POSITIVE
elif self == self.INTRINSIC_LIST_TO_TUPLE:
return OpcodeExecutor_.LIST_TO_TUPLE
else:
raise BreakGraphError(f"No support Intrinsics, {self.name}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

将数据和操作分离吧,数据放在 instr_flag.py,然后在这边进行派发

另外我看了一下背景,大概就是将原本一些低频的字节码集中在了其中两条字节码上(分别是 unary 和 binary),以降低解释器循环大小

那么这里应该只是相当于额外一层派发,不支持的字节码不应该直接 Fallback 么?而不是 BreakGraph

另外也看一下 INTRINSIC_PRINT,根据描述是只用在 REPL 的,可以测试下非 REPL print 是否会有这个字节码,以及是何种行为,如果有,那么我们需要对齐原来 print 的打断行为

Copy link
Member Author

@gouzil gouzil Feb 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INTRINSIC_PRINT仅在交互模式(non-interactive)下触发,且会在inspect.getsourcelines的时候拿不到源码,所以暂时不处理这种行为

@@ -34,3 +35,21 @@ class MAKE_FUNCTION_FLAG:

class CALL_FUNCTION_EX_FLAG:
CFE_HAS_KWARGS = 0x01


class Intrinsics_UnaryFunctions(Enum):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
class Intrinsics_UnaryFunctions(Enum):
class IntrinsicsUnaryFunctions(Enum):

这里按照 Python 代码风格

另外加一下注释,这里的是从 cpython 哪里 copy 来的,以便未来版本适配时更新

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

下一个 PR 加一下 binary 的,但是全部 Fallback 吧

def CALL_INTRINSIC_1(self, instr: Instruction):
assert isinstance(instr.arg, int)
assert instr.arg <= MAX_INTRINSIC_1
opce: OpcodeExecutorBase = self
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个没有必要

Comment on lines 1503 to 1511
def to_func(args):
if args == Intrinsics_UnaryFunctions.INTRINSIC_1_INVALID:
raise RuntimeError("invalid intrinsic function")
elif args == Intrinsics_UnaryFunctions.INTRINSIC_UNARY_POSITIVE:
return opce.UNARY_POSITIVE
elif args == Intrinsics_UnaryFunctions.INTRINSIC_LIST_TO_TUPLE:
return opce.LIST_TO_TUPLE
else:
raise FallbackError(f"No support Intrinsics, {args.name}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不再需要写成函数,直接 if-else 就好了

…or.py

Co-authored-by: Nyakku Shigure <sigure.qaq@gmail.com>
SigureMo
SigureMo previously approved these changes Feb 23, 2024
Copy link
Member

@SigureMo SigureMo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTMeow

SigureMo
SigureMo previously approved these changes Feb 23, 2024
@SigureMo SigureMo self-assigned this Feb 23, 2024
@SigureMo
Copy link
Member

@diadestiny 可以也来 review 下这个~

@SigureMo
Copy link
Member

image

可以通过这个 approve~ @diadestiny

@SigureMo
Copy link
Member

冲突了需要解决下~

@gouzil
Copy link
Member Author

gouzil commented Feb 26, 2024

冲突了需要解决下~

Done

@SigureMo SigureMo merged commit eea10b1 into PaddlePaddle:develop Feb 26, 2024
30 checks passed
@gouzil gouzil deleted the sot_support_CALL_INTRINSIC_1 branch April 23, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants