Page MenuHomePhabricator

[LoongArch] Add support for llvm.trap and llvm.debugtrap
ClosedPublic

Authored by xen0n on Sep 21 2022, 8:26 AM.

Details

Summary

Similar to D69390 for RISCV, use a guaranteed non-existing insn for
llvm.trap and the break insn for llvm.debugtrap.

Diff Detail

Event Timeline

xen0n created this revision.Sep 21 2022, 8:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2022, 8:26 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
xen0n requested review of this revision.Sep 21 2022, 8:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2022, 8:26 AM
xen0n edited the summary of this revision. (Show Details)Sep 21 2022, 8:56 AM
xry111 added a comment.EditedSep 21 2022, 9:04 AM

I have some doubt here: on x86_64 a trap leads to SIGILL, but a debugtrap leads to SIGTRAP. Not sure if they have a good reason or this is just for satisfying some "habit" of x86 programmers.

If we want a SIGILL we can amswap.w $r0, $r0, $r0.

xry111 added a comment.EditedSep 21 2022, 9:13 AM

I have some doubt here: on x86_64 a trap leads to SIGILL, but a debugtrap leads to SIGTRAP. Not sure if they have a good reason or this is just for satisfying some "habit" of x86 programmers.

If we want a SIGILL we can amswap.w $r0, $r0, $r0.

I used amswap.w $r0, $r0, $r1 in my early work. But again I'm not sure if we want a SIGTRAP of SIGILL (or something else) here.

I have some doubt here: on x86_64 a trap leads to SIGILL, but a debugtrap leads to SIGTRAP. Not sure if they have a good reason or this is just for satisfying some "habit" of x86 programmers.

If we want a SIGILL we can amswap.w $r0, $r0, $r0.

I used amswap.w $r0, $r0, $r1 in my early work. But again I'm not sure if we want a SIGTRAP of SIGILL (or something else) here.

Do you mean when rd == rj (but not rd == rk)?

xen0n added a comment.Sep 23 2022, 3:05 AM

I have some doubt here: on x86_64 a trap leads to SIGILL, but a debugtrap leads to SIGTRAP. Not sure if they have a good reason or this is just for satisfying some "habit" of x86 programmers.

If we want a SIGILL we can amswap.w $r0, $r0, $r0.

I used amswap.w $r0, $r0, $r1 in my early work.

It's a shame the ISA manual didn't give us a bit pattern guaranteed to cause INE (hence SIGILL). In theory all-zeros would work but I got told (I believe at least by FlyGoat) that apparently some engineering sample batches of 3A5000 treated the all-zeros insn word as valid, so it's probably better to be extra safe and just avoid it.

But again I'm not sure if we want a SIGTRAP of SIGILL (or something else) here.

I'd really like to differentiate but according to glibc the break 0 is used as the ultimate fallback for abort. Still a shame the two things have to be conflated if strictly implementing the specs...

I have some doubt here: on x86_64 a trap leads to SIGILL, but a debugtrap leads to SIGTRAP. Not sure if they have a good reason or this is just for satisfying some "habit" of x86 programmers.

If we want a SIGILL we can amswap.w $r0, $r0, $r0.

I used amswap.w $r0, $r0, $r1 in my early work. But again I'm not sure if we want a SIGTRAP of SIGILL (or something else) here.

Do you mean when rd == rj (but not rd == rk)?

It's the only way mentioned by the ISA manual to reliably trigger an INE. IIRC on a 3A5000LL amswap.w $r0, $r0, $r0 also works but the manual is some sort of cryptic about the behavior when rd == rj and rd == rk, so I avoided rd == rk.

But again I'm not sure if we want a SIGTRAP of SIGILL (or something else) here.

I'd really like to differentiate but according to glibc the break 0 is used as the ultimate fallback for abort. Still a shame the two things have to be conflated if strictly implementing the specs...

And GCC is generating break 0 for __builtin_trap(), while LLVM translates it to llvm.trap (not debugtrap).

Perhaps using break 0 for both is OK, but I just wonder if there is a better way.

RISC-V uses an undefined instruction for llvm.trap. AArch64 uses different break codes for trap and debugtrap.

I think we need to either request an instruction "must be undefined", or request a documentation of break codes. Note that break 7 is already used for a SIGFPE (when -mcheck-zero-division) and the number 7 is not documented anywhere AFAIK...

xen0n added a comment.Sep 26 2022, 6:17 AM

RISC-V uses an undefined instruction for llvm.trap. AArch64 uses different break codes for trap and debugtrap.

I think we need to either request an instruction "must be undefined", or request a documentation of break codes. Note that break 7 is already used for a SIGFPE (when -mcheck-zero-division) and the number 7 is not documented anywhere AFAIK...

Look at /usr/include/asm/break.h in your LoongArch sysroot for the definitions... inherited from MIPS, of course. Unfortunately there's no reservation about an "abort" kind... maybe I should ask and try to stuff one into Linux before amending the patch to make use of that.

xen0n updated this revision to Diff 467029.Oct 11 2022, 11:45 PM

Adopt @xry111's approach to llvm.trap with guaranteed INE-ing insn.

xen0n edited the summary of this revision. (Show Details)Oct 11 2022, 11:46 PM
SixWeining accepted this revision.Oct 11 2022, 11:58 PM

LGTM. But let's wait @xry111.

This revision is now accepted and ready to land.Oct 11 2022, 11:58 PM

Trap part LGTM. But for other parts I have to admit I don't have enough knowledge to review...

This revision was landed with ongoing or failed builds.Oct 13 2022, 4:28 AM
This revision was automatically updated to reflect the committed changes.