Linux kernel implements WARN() and WARN_ON() asserts using trap instructions. Because gcc __builtin_trap() is not recoverable, Linux Kernel has hand code the trap, at the moment using 'twnei'. This leads to sub-optimal code generation. As the powerpc trap instruction is recoverable as it generated a recoverable exception, it would be extremely usefull to also have a recoverable version of __builtin_trap() in gcc. Maybe call it __buitin_recoverable_trap()
[ Do we not have a keyword for feature requests, btw? I don't see one. ] The only thing needed for GCC is to have a __builtin_trap_no_noreturn (or something with a less horrible name ;-) ), that does exactly that: it's the same as __builtin_trap, just not noreturn. This is useful on most architectures, not just PowerPC.
(In reply to Segher Boessenkool from comment #1) > [ Do we not have a keyword for feature requests, btw? I don't see one. ] Usually enhancement is enough to mark it as a feature request.
Ah, thank you. Well except there is no keyword called that?
'enhancement' Importance is the magic we use, in the end it's a missed optimization since you refer to sub-optimal code gen. I'm not sure what your proposed not noreturn trap() would do in terms of IL semantics compared to a not specially annotated general call? "recoverable" likely means resuming after the trap, not on an exception path (so it'll not be a throw())? The only thing that might be useful to the middle-end would be marking the function as not altering the memory state. But I suppose it should still serve as a barrier for code motion of both loads and stores, even of those loads/stores are known to not trap. The only magic we'd have for this would be __attribute__((const,returns_twice)). Which likely will be more detrimental to general optimization. So - what's the "sub-optimal code generation" you refer to from the (presumably) volatile asm() you use for the trap? [yeah, asm() on GIMPLE is less optimized than a call]
For the naming I suggest __builtin_debugtrap() to align with clang. Maybe with an aliased __debugbreak() on Windows platforms.
(In reply to Richard Biener from comment #4) > I'm not sure what your proposed not noreturn trap() would do in terms of > IL semantics compared to a not specially annotated general call? Nothing I think? But __builtin_trap *is* very different: it ends BBs. > "recoverable" likely means resuming after the trap, not on an exception > path (so it'll not be a throw())? "recoverable" is super unclear. For example, on Power the hardware has a concept "recoverable interrupt", which set MSR[RI]=1, and traps never do. This is a very different concept as what is wanted here, which has nothing to do with recoverability, and is simply about not being an abort() (which __builtin_trap *is*!) > The only thing that might be useful to the middle-end would be marking > the function as not altering the memory state. But I suppose it should > still serve as a barrier for code motion of both loads and stores, even > of those loads/stores are known to not trap. The only magic we'd have > for this would be __attribute__((const,returns_twice)). Which likely > will be more detrimental to general optimization. > > So - what's the "sub-optimal code generation" you refer to from the > (presumably) volatile asm() you use for the trap? > > [yeah, asm() on GIMPLE is less optimized than a call] The rs6000 backend can optimise the used instructions: we have trap_if instructions, both with registers and with immediates. A single instruction can do a comparison and a conditional trap. This works great with __builtin_trap, *if* the kernel's trap handler has abort() semantics. __builtin_trap_no_abort() maybe?
(In reply to Franz Sirl from comment #5) > For the naming I suggest __builtin_debugtrap() to align with clang. Maybe > with an aliased __debugbreak() on Windows platforms. Those are terrible names. This would *not* be used more often than __builtin_trap, for debugging. In general, builtins should say what they *do*, nott what you imagine they will be used for.
(In reply to Segher Boessenkool from comment #7) > (In reply to Franz Sirl from comment #5) > > For the naming I suggest __builtin_debugtrap() to align with clang. Maybe > > with an aliased __debugbreak() on Windows platforms. > > Those are terrible names. This would *not* be used more often than > __builtin_trap, for debugging. > > In general, builtins should say what they *do*, nott what you imagine they > will be used for. Let me re-phrase it. If the result of this discussion will result in a builtin that will be eqivalent to an "int 3" (or was it ud2? on x86) without "noreturn" as it is with clang, I would really prefer it to be called __builtin_debugtrap() even if the name is not great. Having different naming between GCC and clang seems worse to me.
The i386 port has === (define_insn "trap" [(trap_if (const_int 1) (const_int 6))] "" { #ifdef HAVE_AS_IX86_UD2 return "ud2"; #else return ASM_SHORT "0x0b0f"; #endif } [(set_attr "length" "2")]) === which implements __builtin_trap, and can implement __builtin_trap_no_abort just fine as well, if your OS kernel (or similar) can return after a ud2. If clang uses terribly confusing names (or semantics, or syntax, etc.) we should not copy that from them. *Especially* when that already conflicts with names they copied from us.
Dup of bug 84595. *** This bug has been marked as a duplicate of bug 84595 ***
Not a duplicate.