search

python-greenlet / greenlet

Lightweight in-process concurrent programming
Other
1.63k stars 247 forks source link

greenlet 3.0.3 fails to build on riscv64 and ppc64el #395

Open doko42 opened 8 months ago

doko42 commented 8 months ago

greenlet 3.0.3 fails to build on riscv64 and ppc64el:

complete build logs at https://launchpad.net/ubuntu/+source/python-greenlet/3.0.3-0ubuntu2

riscv64-linux-gnu-gcc -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -O2 -fno-omit-frame-pointer -ffile-prefix-map=/<>=. -fstack-protector-strong -Wformat -Werror=format-security -fdebug-prefix-map=/<>=/usr/src/python-greenlet-3.0.3-0ubuntu2 -Wdate-time -D_FORTIFY_SOURCE=3 -fPIC -I/usr/include/python3.11 -c /<>/src/greenlet/greenlet.cpp -o build/temp.linux-riscv64-cpython-311/<>/src/greenlet/greenlet.o -Os In file included from /<>/src/greenlet/slp_platformselect.h:60, from /<>/src/greenlet/greenlet_slp_switch.hpp:64, from /<>/src/greenlet/greenlet.cpp:24: /<>/src/greenlet/platform/switch_riscv_unix.h: In function ‘int slp_switch()’: /<>/src/greenlet/platform/switch_riscv_unix.h:30:1: error: s0 cannot be used in ‘asm’ here 30 | } | ^ /<>/src/greenlet/platform/switch_riscv_unix.h:30:1: error: s0 cannot be used in ‘asm’ here error: command '/usr/bin/riscv64-linux-gnu-gcc' failed with exit code 1

powerpc64le-linux-gnu-gcc -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -fexceptions -g -fwrapv -O2 -g -O3 -fno-omit-frame-pointer -ffile-prefix-map=/<>=. -flto=auto -ffat-lto-objects -fstack-protector-strong -Wformat -Werror=format-security -fdebug-prefix-map=/<>=/usr/src/python-greenlet-3.0.3-0ubuntu2 -Wdate-time -D_FORTIFY_SOURCE=3 -fPIC -I/usr/include/python3.11 -c /<>/src/greenlet/greenlet.cpp -o build/temp.linux-ppc64le-cpython-311/<>/src/greenlet/greenlet.o -fno-tree-dominator-opts In file included from /<>/src/greenlet/slp_platformselect.h:21, from /<>/src/greenlet/greenlet_slp_switch.hpp:64, from /<>/src/greenlet/greenlet.cpp:24: /<>/src/greenlet/platform/switch_ppc64_linux.h: In function ‘slp_switch’: /<>/src/greenlet/platform/switch_ppc64_linux.h:103:1: error: 31 cannot be used in ‘asm’ here 103 | } | ^ /<>/src/greenlet/platform/switch_ppc64_linux.h:103:1: error: 31 cannot be used in ‘asm’ here /<>/src/greenlet/platform/switch_ppc64_linux.h:103:1: error: 31 cannot be used in ‘asm’ here /<>/src/greenlet/platform/switch_ppc64_linux.h:103:1: error: 31 cannot be used in ‘asm’ here error: command '/usr/bin/powerpc64le-linux-gnu-gcc' failed with exit code 1

jamadden commented 8 months ago

PRs welcome.

jamadden commented 8 months ago

Note that riscv is not an officially supported platform, that was a contribution by another contributor and is untested here, so it's not super surprising it doesn't work for you.

The PPC error is interesting, because ppc64le is a supported platform and is tested with every build. Perhaps your compiler is too old?

stefanor commented 8 months ago

Perhaps your compiler is too old?

It's more likely to be the other way around, and too new. gcc 13.2.0-9ubuntu1

xypron commented 5 months ago

s0 is the RISC-V frame-pointer register and the only one not allowed to be marked as clobbered.

Why should we rely on compiler magic (marking registers as clobbered) instead of explicitly pushing registers to the stack and popping them when returning to the thread?

Looking at m68k and arm32 the framepointer register gets special treatment on these platforms too.

xypron commented 5 months ago

PRs welcome.

@jamadden In the documentation I cannot find any description how the tests should be run. Invoking pytest just gives me "ModuleNotFoundError: No module named 'greenlet._greenlet'". ./setup.py test returns "NO TESTS RAN".

doko42 commented 5 months ago

so, the reason that this isn't working, is that recent Linux distros decided to build with -fno-omit-frame-pointer as the default. Apparently both the riscv64 and the ppc64el implementations cannot cope with that.

you should be able to reproduce this by adding -fno-omit-frame-pointer to your build flags

peter-bergner commented 5 months ago

@doko42 Can you email the ppc64le preprocessed source file showing the error and I'll have a look. Thanks.

peter-bergner commented 5 months ago

Nevermind, I was able to create a small reproducer. I'll have a look.

peter-bergner commented 5 months ago

This doesn't seem to be a port specific issue, hence seeing it on both ppc64le and riscv64, but rather a generic register allocation assert that is firing:

void
bug (void)
{
  __asm__ volatile ("" : : : "r31");
}
bergner@ltcden2-lp1:greenlet [master]$ /opt/gcc-nightly/trunk/bin/gcc -S -fno-omit-frame-pointer bug.c 
bug.c: In function ‘bug’:
bug.c:5:1: error: 31 cannot be used in ‘asm’ here
    5 | }
      | ^
bug.c:5:1: error: 31 cannot be used in ‘asm’ here

The code that is asserting is ira.c:ira_setup_eliminable_regset():

/* Build the regset of all eliminable registers and show we can't
     use those that we already know won't be eliminated.  */
  for (i = 0; i < (int) ARRAY_SIZE (eliminables); i++)
    {
      bool cannot_elim
        = (! targetm.can_eliminate (eliminables[i].from, eliminables[i].to)
           || (eliminables[i].to == STACK_POINTER_REGNUM && frame_pointer_needed));

      if (!TEST_HARD_REG_BIT (crtl->asm_clobbers, eliminables[i].from))
        {
            SET_HARD_REG_BIT (eliminable_regset, eliminables[i].from);

            if (cannot_elim)
              SET_HARD_REG_BIT (ira_no_alloc_regs, eliminables[i].from);
        }
      else if (cannot_elim)
        error ("%s cannot be used in %<asm%> here",
               reg_names[eliminables[i].from]);
      else
        df_set_regs_ever_live (eliminables[i].from, true);
    }

On Power, targetm.can_eliminate(r31,r1) returns true (ie, the port will allow us to eliminate r31 into r1) even in the face of -fno-omit-frame-pointer, but it's the RA specific test (eliminables[i].to == STACK_POINTER_REGNUM && frame_pointer_needed) that is catching us here, so the ports don't seem to be at fault here.

I guess the question is, is it legal to specify the frame pointer hard reg in the clobbers set of an asm when using -fno-omit-frame-pointer? I think we need Vlad and some other GCC RA experts to weigh in on that. I'll open a GCC bugzilla and ask that question. I'll note GCC does not emit a predefined macro to tell you whether the user used -fomit-frame-pointer versus -fno-omit-frame-pointer.

peter-bergner commented 5 months ago

I opened GCC PR114664.

snaury commented 5 months ago

You don't want to clobber the frame pointer, because clobbering is just a trick to force the outer function to save and restore all callee saved registers. Let's imagine the compiler would have allowed clobbering the frame pointer, since it uses the frame pointer to access local variables, it would just save and restore it around your asm block, and it wouldn't accomplish anything (technically compilers are allowed to do that for other clobbers, but they usually don't).

Back when working on x86/amd64 I used a trick to both save/restore the frame pointer using a local variable (forced to be allocated on the stack using "m") and shifting it alongside the stack pointer, assuming it could be a frame pointer:

I'm not familiar with these architectures, but this trick is used in several other architectures (e.g. arm32, aarch64 and s390).

peter-bergner commented 5 months ago

The consensus in the GCC bugzilla is that the inline asm as is, is buggy when used with -fno-omit-frame-pointer (it was closed as RESOLVED/INVALID). If arm32/aarch64 and s390 have solved this already, it would be useful to see if we can reuse their solution to this problem.

peter-bergner commented 5 months ago

I notice some architectures are using __attribute__((optimize("no-omit-frame-pointer"))) on their slp_switch() function. As a workaround, might we do the opposite for ppc and riscv and use __attribute__((optimize("omit-frame-pointer"))) instead? I guess I'm not familiar with why the distro build wants to force using -fno-omit-frame-pointer, since it leads to less efficient code on ppc64le. Is this a case of it helps one architecture, so we'll use it everywhere?

xypron commented 5 months ago

https://ubuntu.com/blog/ubuntu-performance-engineering-with-frame-pointers-by-default describes the reasoning for Ubuntu to switch to using frame-pointers.