[COFF, ARM64] Fix encoding of __debugbreak

[TomTan]

Summary

On Windows ARM64, intrinsic __debugbreak should be compiled into brk #0xF000 which is different from llvm intrinsic debugtrap as brk #0. This change fixes this by transforming __debugbreak to the expected inline assembly in Clang.

[rnk]

I think it would be preferable to make llvm.debugtrap emit brk #0xF000 on aarch64-windows-*, so other frontends (Rust etc) get the right behavior by default. Right now, AArch64 doesn't do anything special for DEBUGTRAP, so we get the default behavior of llvm.trap.

Is this BRK #0xF000 convention universal to all OSs, or is it a specific convention for Windows debuggers?

[TomTan]

In D63617#1552561, @rnk wrote:

I think it would be preferable to make llvm.debugtrap emit brk #0xF000 on aarch64-windows-*, so other frontends (Rust etc) get the right behavior by default. Right now, AArch64 doesn't do anything special for DEBUGTRAP, so we get the default behavior of llvm.trap.

Is this BRK #0xF000 convention universal to all OSs, or is it a specific convention for Windows debuggers?

I looked at llvm.debugtrap but I am not sure the side-effect of that fix, like could there be some other ABI (mingw, etc.) on Windows ARM64 which expects DEBUGTRAP to be the same as Android/Linux.

I think this BRK #0xF000 convention is only for Windows debugger and exception handler, such as we define __fastfail in the same way with different operand, not universal for all OSs.

[rnk]

Even if BRK #0xF000 is a Windows convention, it's still possible for ISel to select different instructions for different OSs, and I'd prefer to implement it that way.

[TomTan]

In D63617#1552615, @rnk wrote:

Even if BRK #0xF000 is a Windows convention, it's still possible for ISel to select different instructions for different OSs, and I'd prefer to implement it that way.

Ok, did the implementation in LLVM in https://reviews.llvm.org/D63635.

[TomTan]

The fix in LLVM was merged as https://reviews.llvm.org/rL364115.

lib/CodeGen/CGBuiltin.cpp

RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
   case Builtin::BI__builtin___clear_cache: {
     Value *Begin = EmitScalarExpr(E->getArg(0));
     Value *End = EmitScalarExpr(E->getArg(1));
     Function *F = CGM.getIntrinsic(Intrinsic::clear_cache);
     return RValue::get(Builder.CreateCall(F, {Begin, End}));
   }
   case Builtin::BI__builtin_trap:
     return RValue::get(EmitTrapCall(Intrinsic::trap));
-  case Builtin::BI__debugbreak:
+  case Builtin::BI__debugbreak: {
+    llvm::Triple::ArchType ISA = getTarget().getTriple().getArch();
+    switch (ISA) {
+    default:
       return RValue::get(EmitTrapCall(Intrinsic::debugtrap));
+    case llvm::Triple::aarch64: {
+      llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy, false);
+      llvm::InlineAsm *IA =
+        llvm::InlineAsm::get(FTy, "brk #0xF000", "", /*SideEffects=*/true);
+      llvm::CallInst *CI = Builder.CreateCall(IA);
+      return RValue::get(CI);
+    }
+    }
+  }
   case Builtin::BI__builtin_unreachable: {
     EmitUnreachable(E->getExprLoc());
 
     // We do need to preserve an insertion point.
     EmitBlock(createBasicBlock("unreachable.cont"));
 
     return RValue::get(nullptr);
   }

test/CodeGen/ms-intrinsics.c

 void test__fastfail() {
   __fastfail(42);
 }
 // CHECK-LABEL: define{{.*}} void @test__fastfail()
 // CHECK-ARM: call void asm sideeffect "udf #251", "{r0}"(i32 42) #[[NORETURN:[0-9]+]]
 // CHECK-INTEL: call void asm sideeffect "int $$0x29", "{cx}"(i32 42) #[[NORETURN]]
 // CHECK-ARM64: call void asm sideeffect "brk #0xF003", "{w0}"(i32 42) #[[NORETURN:[0-9]+]]
 
+void test__debugbreak() {
+  __debugbreak();
+}
+// CHECK_LABEL: define{{.*}} void @test__debugbreak() {
+// CHECK-INTEL: call void @llvm.debugtrap()
+// CHECK-ARM: call void @llvm.debugtrap()
+// CHECK-ARM64: call void asm sideeffect "brk #0xF000", ""()
+
 // Attributes come last.
 
 // CHECK: attributes #[[NORETURN]] = { noreturn{{.*}} }