[COFF, ARM64] Fix encoding of debugtrap for Windows

[TomTan]

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.

[rnk]

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.

[efriedma]

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]

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.

[efriedma]

LGTM

llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp

BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(Opc), ResultReg)
       .addReg(SrcReg, getKillRegState(SrcRegIsKill));
     updateValueMap(II, ResultReg);
     return true;
   }
   case Intrinsic::trap:
     BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(AArch64::BRK))
         .addImm(1);
     return true;
+  case Intrinsic::debugtrap: {
+    if (Subtarget->isTargetWindows()) {
+      BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(AArch64::BRK))
+          .addImm(0xF000);
+      return true;
+    }
+    break;
+  }
 
   case Intrinsic::sqrt: {
     Type *RetTy = II->getCalledFunction()->getReturnType();
 
     MVT VT;
     if (!isTypeLegal(RetTy, VT))
       return false;
 

llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp

for (unsigned im = (unsigned)ISD::PRE_INC;
     setIndexedStoreAction(im, MVT::i64, Legal);
     setIndexedStoreAction(im, MVT::f64, Legal);
     setIndexedStoreAction(im, MVT::f32, Legal);
     setIndexedStoreAction(im, MVT::f16, Legal);
   }
 
   // Trap.
   setOperationAction(ISD::TRAP, MVT::Other, Legal);
+  if (Subtarget->isTargetWindows())
+    setOperationAction(ISD::DEBUGTRAP, MVT::Other, Legal);
 
   // We combine OR nodes for bitfield operations.
   setTargetDAGCombine(ISD::OR);
   // Try to create BICs for vector ANDs.
   setTargetDAGCombine(ISD::AND);
 
   // Vector add and sub nodes may conceal a high-half opportunity.
   // Also, try to fold ADD into CSINC/CSINV..

llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td

 def HasCCDP          : Predicate<"Subtarget->hasCCDP()">,
                        AssemblerPredicate<"FeatureCacheDeepPersist", "ccdp">;
 def HasBTI           : Predicate<"Subtarget->hasBTI()">,
                        AssemblerPredicate<"FeatureBranchTargetId", "bti">;
 def HasMTE           : Predicate<"Subtarget->hasMTE()">,
                        AssemblerPredicate<"FeatureMTE", "mte">;
 def IsLE             : Predicate<"Subtarget->isLittleEndian()">;
 def IsBE             : Predicate<"!Subtarget->isLittleEndian()">;
+def IsWindows        : Predicate<"Subtarget->isTargetWindows()">;
 def UseAlternateSExtLoadCVTF32
     : Predicate<"Subtarget->useAlternateSExtLoadCVTF32Pattern()">;
 
 def UseNegativeImmediates
     : Predicate<"false">, AssemblerPredicate<"!FeatureNoNegativeImmediates",
                                              "NegativeImmediates">;
 
 def AArch64LocalRecover : SDNode<"ISD::LOCAL_RECOVER",

 } // AddedComplexity = 20
 
 // To truncate, we can simply extract from a subregister.
 def : Pat<(i32 (trunc GPR64sp:$src)),
           (i32 (EXTRACT_SUBREG GPR64sp:$src, sub_32))>;
 
 // __builtin_trap() uses the BRK instruction on AArch64.
 def : Pat<(trap), (BRK 1)>;
+def : Pat<(debugtrap), (BRK 0xF000)>, Requires<[IsWindows]>;
 
 // Multiply high patterns which multiply the lower subvector using smull/umull
 // and the upper subvector with smull2/umull2. Then shuffle the high the high
 // part of both results together.
 def : Pat<(v16i8 (mulhs V128:$Rn, V128:$Rm)),
           (UZP2v16i8
            (SMULLv8i8_v8i16 (EXTRACT_SUBREG V128:$Rn, dsub),
                             (EXTRACT_SUBREG V128:$Rm, dsub)),

llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp

bool AArch64InstructionSelector::selectIntrinsicWithSideEffects(
 
   // Select the instruction.
   switch (IntrinID) {
   default:
     return false;
   case Intrinsic::trap:
     MIRBuilder.buildInstr(AArch64::BRK, {}, {}).addImm(1);
     break;
+  case Intrinsic::debugtrap:
+    if (!STI.isTargetWindows())
+      return false;
+    MIRBuilder.buildInstr(AArch64::BRK, {}, {}).addImm(0xF000);
+    break;
   case Intrinsic::aarch64_stlxr:
     unsigned StatReg = I.getOperand(0).getReg();
     assert(RBI.getSizeInBits(StatReg, MRI, TRI) == 32 &&
            "Status register must be 32 bits!");
     unsigned SrcReg = I.getOperand(2).getReg();
 
     if (RBI.getSizeInBits(SrcReg, MRI, TRI) != 64) {
       LLVM_DEBUG(dbgs() << "Only support 64-bit sources right now.\n");

llvm/trunk/test/CodeGen/AArch64/windows-trap1.ll

+; RUN: llc -mtriple=aarch64-windows %s -o -| FileCheck %s
+; RUN: llc -mtriple=aarch64-windows -fast-isel %s -o - | FileCheck %s
+; RUN: llc -mtriple=aarch64-windows -global-isel %s -o - | FileCheck %s
+
+; CHECK-LABEL: test1:
+; CHECK: brk #0xf000
+define void @test1() noreturn nounwind  {
+entry:
+  tail call void @llvm.debugtrap( )
+  ret void
+}
+
+declare void @llvm.debugtrap() nounwind