[Bug rtl-optimization/114664] New: -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[Bug rtl-optimization/114664] New: -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

            Bug ID: 114664
           Summary: -fno-omit-frame-pointer causes an ICE during the build
                    of the greenlet package
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: bergner at gcc dot gnu.org
  Target Milestone: ---

Current builds of the greenlet package on one specific distro, are seeing an
ICE on multiple architectures (ppc64le & riscv64) when being built with
-fno-omit-frame-pointer.  The upstream github issue is here:

  https://github.com/python-greenlet/greenlet/issues/395

A minimized test case on Power is:

bergner@ltcden2-lp1:$ cat bug.c 
void
bug (void)
{
  __asm__ volatile ("" : : : "r31");
}
bergner@ltcden2-lp1:$ /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

This is not a regression, as all gcc's I have easy access to (back to gcc v8)
ICE the same way.

The code that is ICEing here is in 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.

The question I have is, is it legal to mention the hard frame pointer register
in an asm clobber list when using -fno-omit-frame-pointer?  Ie, is this user
error or should the compiler be able to handle this?

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Let me find the dups ...

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #2 from Peter Bergner <bergner at gcc dot gnu.org> ---
CC'ing some architecture and RA experts for their input.

I believe the riscv64 test showing the same issue would be:

void
bug (void)
{
  __asm__ volatile ("" : : : "s0");
}

...but I don't have a cross compiler right now to verify.

Interestingly, I tried what I thought would be the aarch64 test case
(clobbering x29), but it did not ICE.  Did I use the wrong hard frame pointer
register or is aarch64 doing something different here?

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Well I am going to say this about the code in that repo, the inline-asm in
slp_switch looks very broken anyways.

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |doko at gcc dot gnu.org,
                   |                            |law at gcc dot gnu.org,
                   |                            |linkw at gcc dot gnu.org,
                   |                            |meissner at gcc dot gnu.org,
                   |                            |rsandifo at gcc dot gnu.org,
                   |                            |segher at gcc dot gnu.org,
                   |                            |vmakarov at gcc dot gnu.org
   Last reconfirmed|                            |2024-04-09
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #4 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> Well I am going to say this about the code in that repo, the inline-asm in
> slp_switch looks very broken anyways.

100% agree, but broken for other reasons.  I think still TBD whether the
minimal test case here is supposed to work or not.

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I wonder why they are not using getcontext/savecontext/swapcontext instead ...

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Pre-IRA fix was done to specifically reject this:
https://inbox.sourceware.org/gcc-patches/ab3a61990702021658w4dc049cap53de8010a7d8626e@mail.gmail.com/

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #7 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #6)
> Pre-IRA fix was done to specifically reject this:
> https://inbox.sourceware.org/gcc-patches/
> ab3a61990702021658w4dc049cap53de8010a7d8626e@mail.gmail.com/

Then that would seem to indicate that mentioning the frame pointer reg in the
asm clobber list is an error, but how are users supposed to know whether
-fno-omit-frame-pointer is in effect or not?  I've looked and there is no
pre-defined macro a user could check.

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #8 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to Peter Bergner from comment #7)
> (In reply to Andrew Pinski from comment #6)
> > Pre-IRA fix was done to specifically reject this:
> > https://inbox.sourceware.org/gcc-patches/
> > ab3a61990702021658w4dc049cap53de8010a7d8626e@mail.gmail.com/
> 
> Then that would seem to indicate that mentioning the frame pointer reg in
> the asm clobber list is an error, but how are users supposed to know whether
> -fno-omit-frame-pointer is in effect or not?  I've looked and there is no
> pre-defined macro a user could check.

I noticed even without -fno-omit-frame-pointer, the test case still fails with
the same symptom (with error msg rather than ICE), did I miss something?

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #9 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to Kewen Lin from comment #8)
> I noticed even without -fno-omit-frame-pointer, the test case still fails
> with the same symptom (with error msg rather than ICE), did I miss something?

With no option, we default to -fomit-frame-pointer and that option does not
guarantee we actually will omit the frame pointer.

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #10 from Richard Sandiford <rsandifo at gcc dot gnu.org> ---
(In reply to Peter Bergner from comment #7)
> Then that would seem to indicate that mentioning the frame pointer reg in
> the asm clobber list is an error
Yeah, I agree it's an error.  The PR says “ICE”, but is there an internal
error?  The “cannot be used in ‘asm’ here” is a normal user-facing error,
albeit with bad error recovery, leading us to report the same thing multiple
times.

> but how are users supposed to know whether
> -fno-omit-frame-pointer is in effect or not?  I've looked and there is no
> pre-defined macro a user could check.
That might be a useful thing to have, but if the programmer has no control over
the build flags (i.e. cannot require/force -fomit-frame-pointer) then I think
the asm has to take care to save and restore the frame pointer itself.

Dropping "31" from the asm means that the asm must preserve the register. 
Things will go badly if the asm doesn't do that.

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #11 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to Richard Sandiford from comment #10)
> Yeah, I agree it's an error.  The PR says “ICE”, but is there an internal
> error?  The “cannot be used in ‘asm’ here” is a normal user-facing error,
> albeit with bad error recovery, leading us to report the same thing multiple
> times.

My bad for calling it an ICE.  Clearly it's not an ICE but a normal error as
you say.



> > but how are users supposed to know whether
> > -fno-omit-frame-pointer is in effect or not?  I've looked and there is no
> > pre-defined macro a user could check.
> That might be a useful thing to have, but if the programmer has no control
> over the build flags (i.e. cannot require/force -fomit-frame-pointer) then I
> think the asm has to take care to save and restore the frame pointer itself.
> 
> Dropping "31" from the asm means that the asm must preserve the register. 
> Things will go badly if the asm doesn't do that.

So r31 which we use as our frame-pointer reg is a non-volatile/callee saved
register, so it must be saved, but I guess they (greenlet) cannot use the
method of mentioning it in the asm clobber list to perform that.

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #12 from Richard Sandiford <rsandifo at gcc dot gnu.org> ---
(In reply to Peter Bergner from comment #11)
> > > but how are users supposed to know whether
> > > -fno-omit-frame-pointer is in effect or not?  I've looked and there is no
> > > pre-defined macro a user could check.
> > That might be a useful thing to have, but if the programmer has no control
> > over the build flags (i.e. cannot require/force -fomit-frame-pointer) then I
> > think the asm has to take care to save and restore the frame pointer itself.
> > 
> > Dropping "31" from the asm means that the asm must preserve the register. 
> > Things will go badly if the asm doesn't do that.
> 
> So r31 which we use as our frame-pointer reg is a non-volatile/callee saved
> register, so it must be saved, but I guess they (greenlet) cannot use the
> method of mentioning it in the asm clobber list to perform that.
I was thinking of just the asm in isolation, rather than its effect on the
containing function's callee-save set.

If you have:

  asm volatile ("…");

then GCC expects r31 after the asm to equal r31 before the asm, regardless of
the -fomit-frame-pointer setting.  If the asm is:

  asm volatile ("li r31,0");

(sorry, I've forgotten the actual Power asm :)) then things will go wrong if
GCC tries to use r31 after the asm.

If the asm clobbers r31 then it must mention it in the clobber list.  As things
stand, it's not possible to mention r31 in the clobber list if
-fno-omit-frame-pointer.  This means that the only option for code that wants
to support -fno-omit-frame-pointer is to make the asm's contents preserve r31,
using an explicit save and restore if necessary.

And that kind-of makes sense.  If an asm clobbers the frame pointer, that
removes GCC's main option for restoring data after the asm.  A lot of other
clobbers would be handled by loading data relative to the frame pointer.  If
the frame pointer itself has gone then things get tricky.

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #13 from Peter Bergner <bergner at gcc dot gnu.org> ---
So I think the conclusion is we should close this as INVALID, correct?

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #14 from Richard Sandiford <rsandifo at gcc dot gnu.org> ---
Yeah, I think so.

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |INVALID

--- Comment #15 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to Richard Sandiford from comment #14)
> Yeah, I think so.

Ok, then marking as INVALID and greenlet will need to come up with some other
solution than the one they're using.

[Bug rtl-optimization/114664] -fno-omit-frame-pointer causes an ICE during the build of the greenlet package

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114664

--- Comment #16 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Yup, GPR31 is used for the emulated frame pointer, so this is user error:
saying
a fixed-purpose register is clobbered makes no sense.  You are not allowed to
use any register that the compiler uses for function calling any other way.

It is a bad idea to use -fno-omit-frame-pointer on Power, btw, just like on any
other architecture that does not have a frame pointer architecturally.  We
don't
have one in any ABI even (for C at least, who knows what other languages (or
language implementations) do).  It is a grudge for targets that do not generate
proper DWARF, or you can use it to quickly validate some (suspected!) bug is
not
in certain parts of the compiler; but we do not want to ever use it by default,
esp. because it is so costly.  Benchmark it, and quickly be dissuaded from even
thinking about ever doing this.  Progress is Good.