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

greenlet 3.0.3 fails to build on riscv64 and ppc64el #395

Open
doko42 opened this issue Jan 3, 2024 · 14 comments
Open

greenlet 3.0.3 fails to build on riscv64 and ppc64el #395

doko42 opened this issue Jan 3, 2024 · 14 comments

Comments

@doko42
Copy link

doko42 commented Jan 3, 2024

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
Copy link
Contributor

jamadden commented Jan 8, 2024

PRs welcome.

@jamadden
Copy link
Contributor

jamadden commented Jan 8, 2024

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
Copy link

stefanor commented Jan 8, 2024

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
Copy link
Contributor

xypron commented Apr 2, 2024

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
Copy link
Contributor

xypron commented Apr 2, 2024

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
Copy link
Author

doko42 commented Apr 2, 2024

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
Copy link

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

@peter-bergner
Copy link

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

@peter-bergner
Copy link

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
Copy link

I opened GCC PR114664.

@snaury
Copy link
Contributor

snaury commented Apr 10, 2024

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:

  • When it is indeed a frame pointer, saving/restoring doesn't change the value. Since memory pointers are relative to the frame pointer shifting makes it point to the new location after the stack is restored.
  • When it is not a frame pointer, we are corrupting some random register which might be used by the compiler for something. But there's not much the compiler could use it for, and we immediately call slp_restore_state to restore the stack, after which the stack has the correct value of the non-frame-pointer register and we restore it.

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

@peter-bergner
Copy link

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
Copy link

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
Copy link
Contributor

xypron commented Apr 10, 2024

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

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

No branches or pull requests

6 participants