Skip to content

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让我测测)

@paddle-bot
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的时候拿不到源码,所以暂时不处理这种行为

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
@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

3 participants