bpo-46498: Add new triplets for loongarch64 #2

loongson-zn · 2022-01-26T04:48:56Z

#1 (comment) 根据新世界规范，重新提交补丁。由于上游最近合的较多，重新创建了分支 @xen0n @loongarch64/dev-team
configure为autoconf生成

xen0n · 2022-01-26T05:29:57Z

这个改法是我提的，commit message 加上 Suggested by WANG Xuerui 可

loongson-zn · 2022-01-26T05:57:48Z

这个改法是我提的，commit message 加上 Suggested by WANG Xuerui 可

当然

loongson-zn · 2022-01-26T06:01:46Z

在看一下还没有问题，如果可以的话，希望下午就将此commit提交，cpython更新的速度太快，托的太久还要重新整理提交 @xen0n @yetist @zhuyaliang

xen0n

commit message 每行不需要加星号（别人加是因为他们想表达一个列表）

otherwise LGTM

Suggested by WANG Xuerui

zhuyaliang

good

yetist · 2022-01-27T07:15:47Z

#1 (comment) 根据新世界规范，重新提交补丁。由于上游最近合的较多，重新创建了分支 @xen0n @loongarch64/dev-team configure为autoconf生成

没必要重新创建分支呀，可以在原来那个分支上继续修改，然后 push --force 就可以继续讨论了。

yetist · 2022-01-27T07:25:05Z

configure.ac

@@ -884,6 +884,20 @@ cat >> conftest.c <<EOF
        hppa-linux-gnu
 # elif defined(__ia64__)
        ia64-linux-gnu
+# elif defined(__loongarch__)


不如干脆把 loongarch32 也写上，或者把最外层的 __loongarch__ 去掉？

loongarch32 的宏定义在规范文档里没写，所以暂时没加

__loongarch_xxx_float 这 3 个应该不分 32、64 吧？

__loongarch_xxx_float 这 3 个应该不分 32、64 吧？

看文档，这仨只表达浮点 ABI，整数 ABI 是 __loongarchXX 表达的，这样写应该没问题

yetist · 2022-01-27T07:28:52Z

https://bugs.python.org/issue46498#msg411701 这里的讨论指向的是 #1, 现在把 #1 给 close 了，这不是有点尴尬了？

#1 (comment) 根据新世界规范，重新提交补丁。由于上游最近合的较多，重新创建了分支 @xen0n @loongarch64/dev-team configure为autoconf生成

没必要重新创建分支呀，可以在原来那个分支上继续修改，然后 push --force 就可以继续讨论了。

yetist · 2022-01-27T07:36:56Z

另外 46498 上附加的补丁显然不对。

loongson-zn · 2022-01-27T08:03:06Z

另外 46498 上附加的补丁显然不对。

https://bugs.python.org/issue46498 补丁将会进行更新

yetist

那就先这样吧，rebase一下把补丁更新了

xen0n

我看可以了

xen0n · 2022-01-28T02:06:21Z

虽然现在 3 个赞了，不要直接合，听 @yetist 先 rebase 一下准没错

loongson-zn · 2022-02-09T01:21:42Z

happy new year！开工大吉！
节前用个人github账号已提交，不需要rebase ，直接merge到la64分支就可以了！ @xen0n @yetist
python#30939 (comment)

yetist · 2022-04-28T09:19:37Z

我看上游还没有合并呢，要不要给这里再加一个补丁：

cpython/Modules/_ctypes/callproc.c

Lines 1114 to 1116 in 7cf285d

    
           #if (defined(__x86_64__) && (defined(__MINGW64__) || defined(__CYGWIN__))) || \ 
        
               defined(__aarch64__) || defined(__riscv) 
        
           #define CTYPES_PASS_BY_REF_HACK

xen0n · 2022-04-28T09:25:31Z

不是很确定，参考下当时 RV 修他们问题的上下文，跑下测试？如果 LA 上也是挂的，那就也要带上，否则不用了

cpython/Misc/NEWS.d/3.8.0a1.rst

Lines 1794 to 1800 in 7cf285d

    
           .. bpo: 35847 
        
           .. date: 2019-01-29-09-11-09 
        
           .. nonce: eiSi4t 
        
           .. section: Library 
        
           RISC-V needed the CTYPES_PASS_BY_REF_HACK.  Fixes ctypes Structure 
        
           test_pass_by_value.

…python#91466) Fix an uninitialized bool in exception print context. `struct exception_print_context.need_close` was uninitialized. Found by oss-fuzz in a test case running under the undefined behavior sanitizer. https://oss-fuzz.com/testcase-detail/6217746058182656 ``` Python/pythonrun.c:1241:28: runtime error: load of value 253, which is not a valid value for type 'bool' #0 0xbf2203 in print_chained cpython3/Python/pythonrun.c:1241:28 #1 0xbea4bb in print_exception_cause_and_context cpython3/Python/pythonrun.c:1320:19 #2 0xbea4bb in print_exception_recursive cpython3/Python/pythonrun.c:1470:13 python#3 0xbe9e39 in _PyErr_Display cpython3/Python/pythonrun.c:1517:9 ``` Pretty obvious what the ommission was upon code inspection.

xry111 · 2022-04-28T09:54:01Z

不是很确定，参考下当时 RV 修他们问题的上下文，跑下测试？如果 LA 上也是挂的，那就也要带上，否则不用了

cpython/Misc/NEWS.d/3.8.0a1.rst

Lines 1794 to 1800 in 7cf285d

.. bpo: 35847

.. date: 2019-01-29-09-11-09

.. nonce: eiSi4t

.. section: Library

RISC-V needed the CTYPES_PASS_BY_REF_HACK. Fixes ctypes Structure

test_pass_by_value.

我也不是很确定，前两天看 libffi 的 PR 感觉理论上我们也需要这个东西，但是我跑 test 是好的。

xen0n · 2022-04-28T09:58:19Z

不是很确定，参考下当时 RV 修他们问题的上下文，跑下测试？如果 LA 上也是挂的，那就也要带上，否则不用了

cpython/Misc/NEWS.d/3.8.0a1.rst

Lines 1794 to 1800 in 7cf285d

.. bpo: 35847

.. date: 2019-01-29-09-11-09

.. nonce: eiSi4t

.. section: Library

RISC-V needed the CTYPES_PASS_BY_REF_HACK. Fixes ctypes Structure

test_pass_by_value.

我也不是很确定，前两天看 libffi 的 PR 感觉_理论上_我们也需要这个东西，但是我跑 test 是好的。

跟我感受差不多。。他那个代码片段完整是：

cpython/Modules/_ctypes/callproc.c

Lines 1114 to 1119 in 7cf285d

    
           #if (defined(__x86_64__) && (defined(__MINGW64__) || defined(__CYGWIN__))) || \ 
        
               defined(__aarch64__) || defined(__riscv) 
        
           #define CTYPES_PASS_BY_REF_HACK 
        
           #define POW2(x) (((x & ~(x - 1)) == x) ? x : 0) 
        
           #define IS_PASS_BY_REF(x) (x > 8 || !POW2(x)) 
        
           #endif

控制的行为在：

cpython/Modules/_ctypes/callproc.c

Lines 1238 to 1257 in 7cf285d

    
               for (i = 0; i < argcount; ++i) { 
        
                   atypes[i] = args[i].ffi_type; 
        
           #ifdef CTYPES_PASS_BY_REF_HACK 
        
                   size_t size = atypes[i]->size; 
        
                   if (IS_PASS_BY_REF(size)) { 
        
                       void *tmp = alloca(size); 
        
                       if (atypes[i]->type == FFI_TYPE_STRUCT) 
        
                           memcpy(tmp, args[i].value.p, size); 
        
                       else 
        
                           memcpy(tmp, (void*)&args[i].value, size); 
        
                       avalues[i] = tmp; 
        
                   } 
        
                   else 
        
           #endif 
        
                   if (atypes[i]->type == FFI_TYPE_STRUCT) 
        
                       avalues[i] = (void *)args[i].value.p; 
        
                   else 
        
                       avalues[i] = (void *)&args[i].value; 
        
               }

打开 CTYPES_PASS_BY_REF_HACK 的意思是，对于大于 8 字节或者不是 2 的整数次幂的参数，会按引用传递；很明显 LA ABI 从头到尾是没提过这种奇怪要求的。。

xry111 · 2022-04-28T10:59:34Z

打开 CTYPES_PASS_BY_REF_HACK 的意思是，对于大于 8 字节或者不是 2 的整数次幂的参数，会按引用传递；很明显 LA ABI 从头到尾是没提过这种奇怪要求的。。

它其实说的是对于大于 8 字节或者不是 2 的整数次幂的东西会复制一下再进行传递。这是因为 libffi 的文档里面说 libffi 的“按值传递”是假的，比如一个 32 字节的结构体我们直接扔给 libffi 去传递，等函数返回以后我们可能发现结构体的值变了：

https://github.com/libffi/libffi/blob/4e07374c2773711902ec12905d5c64d95d22d050/doc/libffi.texi#L229

现在 libffi 的情况我认为让他们搞得非常混乱。之前有人为 ARM64 修了这个问题，但后来又撤回了，因为毕竟按文档来说这是“feature”，不是“bug”：

libffi/libffi@482b37f
libffi/libffi@e66fd67

但是又有人加了个 test，似乎暗含的意思是按值传递“应该”正常工作：

libffi/libffi@43e4ad4

于是我在 libffi 的 PR 里面 at 维护者去问，结果人家不鸟我……

xry111 · 2022-05-09T10:46:26Z

我看上游还没有合并呢，要不要给这里再加一个补丁：

cpython/Modules/_ctypes/callproc.c

Lines 1114 to 1116 in 7cf285d

#if (defined(__x86_64__) && (defined(__MINGW64__) || defined(__CYGWIN__))) || \

defined(__aarch64__) || defined(__riscv)

#define CTYPES_PASS_BY_REF_HACK

libffi 那边的最新回复是这个问题在 libffi 一侧解决，Python 侧不用加。

loongson-zn mentioned this pull request Jan 26, 2022

Add new triplets for loongarch64 #1

Closed

loongson-zn force-pushed the dev-main01 branch from a89b10f to a75075e Compare January 26, 2022 05:58

xen0n reviewed Jan 26, 2022

View reviewed changes

bpo-46498: Add new triplets for loongarch64

1566f72

Suggested by WANG Xuerui

loongson-zn force-pushed the dev-main01 branch from a75075e to 1566f72 Compare January 26, 2022 06:15

zhuyaliang approved these changes Jan 27, 2022

View reviewed changes

yetist reviewed Jan 27, 2022

View reviewed changes

yetist approved these changes Jan 28, 2022

View reviewed changes

xen0n approved these changes Jan 28, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-46498: Add new triplets for loongarch64 #2

bpo-46498: Add new triplets for loongarch64 #2

loongson-zn commented Jan 26, 2022 •

edited

Loading

xen0n commented Jan 26, 2022

loongson-zn commented Jan 26, 2022

loongson-zn commented Jan 26, 2022

xen0n left a comment

zhuyaliang left a comment

yetist commented Jan 27, 2022

yetist Jan 27, 2022 •

edited

Loading

xen0n Jan 27, 2022

yetist Jan 27, 2022 •

edited

Loading

xen0n Jan 28, 2022

yetist commented Jan 27, 2022

yetist commented Jan 27, 2022

loongson-zn commented Jan 27, 2022

yetist left a comment

xen0n left a comment

xen0n commented Jan 28, 2022

loongson-zn commented Feb 9, 2022

yetist commented Apr 28, 2022

xen0n commented Apr 28, 2022

xry111 commented Apr 28, 2022

xen0n commented Apr 28, 2022 •

edited

Loading

xry111 commented Apr 28, 2022 •

edited

Loading

xry111 commented May 9, 2022

bpo-46498: Add new triplets for loongarch64 #2

Are you sure you want to change the base?

bpo-46498: Add new triplets for loongarch64 #2

Conversation

loongson-zn commented Jan 26, 2022 • edited Loading

xen0n commented Jan 26, 2022

loongson-zn commented Jan 26, 2022

loongson-zn commented Jan 26, 2022

xen0n left a comment

Choose a reason for hiding this comment

zhuyaliang left a comment

Choose a reason for hiding this comment

yetist commented Jan 27, 2022

yetist Jan 27, 2022 • edited Loading

Choose a reason for hiding this comment

xen0n Jan 27, 2022

Choose a reason for hiding this comment

yetist Jan 27, 2022 • edited Loading

Choose a reason for hiding this comment

xen0n Jan 28, 2022

Choose a reason for hiding this comment

yetist commented Jan 27, 2022

yetist commented Jan 27, 2022

loongson-zn commented Jan 27, 2022

yetist left a comment

Choose a reason for hiding this comment

xen0n left a comment

Choose a reason for hiding this comment

xen0n commented Jan 28, 2022

loongson-zn commented Feb 9, 2022

yetist commented Apr 28, 2022

xen0n commented Apr 28, 2022

xry111 commented Apr 28, 2022

xen0n commented Apr 28, 2022 • edited Loading

xry111 commented Apr 28, 2022 • edited Loading

xry111 commented May 9, 2022

loongson-zn commented Jan 26, 2022 •

edited

Loading

yetist Jan 27, 2022 •

edited

Loading

yetist Jan 27, 2022 •

edited

Loading

xen0n commented Apr 28, 2022 •

edited

Loading

xry111 commented Apr 28, 2022 •

edited

Loading