Skip to content

[SOT][3.12] Eval frame compat for Python 3.12 - #61272

Merged
gouzil merged 10 commits into
PaddlePaddle:developfrom
gouzil:eval_frame_py312
Jan 29, 2024
Merged

[SOT][3.12] Eval frame compat for Python 3.12#61272
gouzil merged 10 commits into
PaddlePaddle:developfrom
gouzil:eval_frame_py312

Conversation

@gouzil

@gouzil gouzil commented Jan 29, 2024

Copy link
Copy Markdown
Member

PR types

Others

PR changes

Others

Description

尝试适配 sot Python3.12 (还没在ci打开单测)

@gouzil
gouzil requested review from SigureMo and removed request for SigureMo January 29, 2024 04:37
Comment thread paddle/fluid/pybind/cpython_internals.c Outdated
Comment on lines +449 to +450
#if PY_VERSION_HEX >= 0x030c0000
#else

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
#if PY_VERSION_HEX >= 0x030c0000
#else
#if PY_VERSION_HEX < 0x030c0000

Py_INCREF(func);
#if PY_VERSION_HEX < 0x030c0000
#if PY_VERSION_HEX >= 0x030c0000
Py_XINCREF(((PyFunctionObject *)frame->f_funcobj)->func_closure);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这里确定要强转才可以么?原来类型不是这个么?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

frame->f_funcobj 已经是 PyObject 类型了,或许可以直接写Py_XINCREF(frame->f_funcobj);

#if PY_VERSION_HEX >= 0x030c0000
Py_XINCREF(((PyFunctionObject *)frame->f_funcobj)->func_closure);
func->func_closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure;
_PyFrame_Initialize(shadow, func, NULL, code, 0);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

最后一个参数确定是 0 么?

@gouzil gouzil Jan 29, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

在初始化 shadow 的时候,希望把所有 localsplus 初始化为 NULL ,所以他是0,_PyFrame_Initialize 会在函数内部根据 code->co_nlocalsplus 的大小逐个赋 NULL

SigureMo
SigureMo previously approved these changes Jan 29, 2024

@SigureMo SigureMo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这两个文件下一个 PR 修改

def gen_call_function(self, argc=0):
if sys.version_info >= (3, 11):
self._add_instr("PRECALL", arg=argc, argval=argc)
if sys.version_info == (3, 11):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if sys.version_info == (3, 11):
if sys.version_info >= (3, 11) and sys.version_info < (3, 12):
Python 3.11.6 (main, Oct  2 2023, 13:45:54) [Clang 15.0.0 (clang-1500.0.40.1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.version_info
sys.version_info(major=3, minor=11, micro=6, releaselevel='final', serial=0)
>>> sys.version_info == (3, 11)
False
>>> (3, 11) <= sys.version_info < (3, 12)
True
>>> sys.version_info >= (3, 11) and sys.version_info < (3, 12)
True
>>> 

虽然 (3, 11) <= sys.version_info < (3, 12) 看起来更好些,但静态分析工具不支持这种 pattern

Comment on lines +725 to +746
if sys.version_info >= (3, 12) and instr.arg & 1:
attr_name_var = ConstantVariable.wrap_literal(
attr_name, self._graph
)
obj = self.stack.pop()

method = BuiltinVariable(
getattr, graph=self._graph, tracker=DanglingTracker()
)(obj, attr_name_var)

if isinstance(method, MethodVariable) and "__getattr__" not in dir(
method.bound_instance.get_py_type()
):
# bound method or the class override the __getattr__
# push the unbound method and the self
self.stack.push(method.fn)
self.stack.push(obj)
else:
# unbound method, push the dummy and the function
self.stack.push(NullVariable())
self.stack.push(method)
return

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这是 LOAD_METHOD 是吧,可以看看怎么复用,比如抽出一个新的函数 load_method

@SigureMo

Copy link
Copy Markdown
Member

点错了,咋直接 approve 了

version_info.minor <= 8 or version_info.minor >= 13
):
# print("skip test_eval_frame, current only support 3.8 - 3.10")
# print("skip test_eval_frame, current only support 3.8 - 3.13")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这行直接删掉

(不然还触发 print 检查)

def test_eval_frame(self):
if version_info.major != 3 or (
version_info.minor <= 8 or version_info.minor >= 12
version_info.minor <= 8 or version_info.minor >= 13

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

原来写的 <= 8 不对吧?

统一用

if not (sys.version_info >= (3, 8) and sys.version_info < (3, 13)):

这种 pattern 吧

@SigureMo SigureMo changed the title [SOT] try support Python3.12 [SOT][3.12] Eval frame compat for Python 3.12 Jan 29, 2024

@SigureMo SigureMo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTMeow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants