Page MenuHomePhabricator

[COFF, ARM64] Fix encoding of debugtrap for Windows
ClosedPublic

Authored by TomTan on Thu, Jun 20, 5:54 PM.

Details

Summary

On Windows ARM64, intrinsic __debugbreak is compiled into brk #0xF000 which is mapped to llvm.debugtrap in Clang. Instruction brk #F000 is the defined break point instruction on ARM64 which is recognized by Windows debugger and exception handling code, so llvm.debugtrap should map to it instead of redirecting to llvm.trap (brk #1) as the default implementation.

Diff Detail

Repository
rL LLVM

Event Timeline

TomTan created this revision.Thu, Jun 20, 5:54 PM
rnk added a comment.Fri, Jun 21, 8:42 AM

I think fastisel will simply fall back to SDAG, which is fine, but llc -global-isel on this test case aborts, so I think you need to handle this next to Intrinsic::trap in AArch64InstructionSelector::selectIntrinsicWithSideEffects as well. I could be wrong, though, I haven't worked with globalisel. I believe it's the default strategy for O0 AArch64 codegen these days, so it would be good to be able to select this intrinsic. Otherwise, looks good, thanks for handling this in LLVM.

test/CodeGen/AArch64/trap.ll
1 ↗(On Diff #205934)

Can you add RUN lines to test this with fast and global isel? Just re-run with -fast-isel and -global-isel using the same CHECKs.

I believe it's the default strategy for O0 AArch64 codegen these days

The configuration we use for that has -global-isel-abort=0, so it will also fall back to SelectionDAG isel. So it shouldn't be necessary to implement it, although it might be nice as a followup.

TomTan updated this revision to Diff 206076.Fri, Jun 21, 2:37 PM

Thanks Reid and Eli for the catch. Added -fast-isel and -global-isel to test configurations, and also added the implementation for both fast isel and global isel.

This revision is now accepted and ready to land.Fri, Jun 21, 2:40 PM
This revision was automatically updated to reflect the committed changes.