On 7/21/25 5:59 PM, Andrew Pinski wrote:
On Mon, Jul 21, 2025 at 4:46 PM Jeff Law <jeffreya...@gmail.com> wrote:
On 7/19/25 2:22 PM, Andrew Pinski wrote:
When we have an empty function, things can go wrong with
cfi_startproc/cfi_endproc and a few other things like exceptions. So if
the only thing the function does is a call to __builtin_unreachable,
let's replace that with a __builtin_trap instead if the target has a trap
instruction. For targets without a trap instruction defined, replace it
with an infinite loop; this allows not to need for the abort call to happen
but still get the correct behavior of not having two functions at the same
location.
The QOI idea for basic block reorder is recorded as PR 120004.
Changes since v1:
* v2: Move to final gimple cfg cleanup instead of expand and use
BUILT_IN_UNREACHABLE_TRAP.
* v3: For targets without a trap defined, create an infinite loop.
Bootstrapped and tested on x86_64-linux-gnu.
PR middle-end/109267
gcc/ChangeLog:
* tree-cfgcleanup.cc (execute_cleanup_cfg_post_optimizing): If the first
non debug statement in the first (and only) basic block is a call
to __builtin_unreachable change it to a call to __builtin_trap or an
infinite loop.
gcc/testsuite/ChangeLog:
* lib/target-supports.exp (check_effective_target_trap): New proc.
* g++.dg/missing-return.C: Update testcase for the !trap case.
* gcc.dg/pr109267-1.c: New test.
* gcc.dg/pr109267-2.c: New test.
For better or worse __builtin_unreachable doesn't halt the program, it
just lets it keep going in an undefined state. A NOP seems like the
choice that would preserve behavior here.
Is there a reason you went with trapping/infinite loop? I think that's
a better solution all-around for __builtin_unreachable, but I'm pretty
sure I'm in the minority on that opinion.
For a nop would require changing all backends to support a
volatile_nop pattern which I was not willing to do. I did try using
the nop pattern to get a nop but with optimizations turned on the nop
would be removed so it was not useful at all. This is why I stuck with
trap/infinite loop. I had mentioned this in the v2 of the patch.
Ah. I don't guess there's a generic way to mark it as volatile thus the
need to fixup all the backends. Sigh.
Richard B. suggested infinite loop in response to v2 of the patch
where I mentioned I tried nop:
https://inbox.sourceware.org/gcc-patches/CAFiYyc0sxrRDB982kHXah=+RstwMckXsrkOnhcAo69cdYU5=w...@mail.gmail.com/
.
Also trap is used by both the x86 and powerpc backend for darwin when
the function is empty already. See r7-4921-g03f82a6a634ddf which
implements the workaround for x86 and ppc backends for darwin only.
ACK. With that out of the way I'll take a look ASAP, unless Richard B.
beats me to it.
jeff
