AArch64: Base MCAsmInfo type on binary format before OS

[arsenm]

Fixes asserting with windows-elf triples. Should fix regression
reported in #147225 (comment)

I'm not sure this is a valid triple, but I'm guessing the MCJIT usage
is an accident. This does change the behavior from trying to use WinEH
to DwarfCFI; however the backend crashes with WinEH so I'm assuming following
ELF is the more correct option.

[arsenm]

• AArch64: Fix asserting on unexpected triples #147880
• AArch64: Base MCAsmInfo type on binary format before OS #147875 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-backend-aarch64

Author: Matt Arsenault (arsenm)

Changes

Fixes asserting with windows-elf triples. Should fix regression
reported in #147225 (comment)

I'm not sure this is a valid triple, but I'm guessing the MCJIT usage
is an accident. This does change the behavior from trying to use WinEH
to DwarfCFI; however the backend crashes with WinEH so I'm assuming following
ELF is the more correct option.

---

Full diff: https://github.com/llvm/llvm-project/pull/147875.diff

3 Files Affected:

• (modified) llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp (+4-4)
• (added) llvm/test/CodeGen/AArch64/exception-handling-windows-elf.ll (+42)
• (modified) llvm/unittests/TargetParser/TripleTest.cpp (+4)

diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp
index efc13589bab63..f918e3cbc7b80 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp
@@ -349,14 +349,14 @@ static MCAsmInfo *createAArch64MCAsmInfo(const MCRegisterInfo &MRI,
   MCAsmInfo *MAI;
   if (TheTriple.isOSBinFormatMachO())
     MAI = new AArch64MCAsmInfoDarwin(TheTriple.getArch() == Triple::aarch64_32);
+  else if (TheTriple.isOSBinFormatELF())
+    MAI = new AArch64MCAsmInfoELF(TheTriple);
   else if (TheTriple.isWindowsMSVCEnvironment())
     MAI = new AArch64MCAsmInfoMicrosoftCOFF();
   else if (TheTriple.isOSBinFormatCOFF())
     MAI = new AArch64MCAsmInfoGNUCOFF();
-  else {
-    assert(TheTriple.isOSBinFormatELF() && "Invalid target");
-    MAI = new AArch64MCAsmInfoELF(TheTriple);
-  }
+  else
+    llvm_unreachable("Invalid target"); // FIXME: This is not unreachable
 
   // Initial state of the frame pointer is SP.
   unsigned Reg = MRI.getDwarfRegNum(AArch64::SP, true);
diff --git a/llvm/test/CodeGen/AArch64/exception-handling-windows-elf.ll b/llvm/test/CodeGen/AArch64/exception-handling-windows-elf.ll
new file mode 100644
index 0000000000000..f7ddda9815c99
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/exception-handling-windows-elf.ll
@@ -0,0 +1,42 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=aarch64-pc-windows-elf < %s | FileCheck %s
+; Make sure windows elf does not assert with exceptions
+
+@_ZTIi = external global ptr
+
+declare i32 @foo(i32)
+declare i32 @__gxx_personality_v0(...)
+
+define void @bar() personality ptr @__gxx_personality_v0 {
+; CHECK-LABEL: bar:
+; CHECK:       // %bb.0: // %continue
+; CHECK-NEXT:    sub sp, sp, #32
+; CHECK-NEXT:    str x30, [sp, #16] // 8-byte Folded Spill
+; CHECK-NEXT:    .cfi_def_cfa_offset 32
+; CHECK-NEXT:    .cfi_offset w30, -16
+; CHECK-NEXT:    adrp x8, :got:foo
+; CHECK-NEXT:    mov w0, #42 // =0x2a
+; CHECK-NEXT:    ldr x8, [x8, :got_lo12:foo]
+; CHECK-NEXT:    blr x8
+; CHECK-NEXT:    ldr x30, [sp, #16] // 8-byte Folded Reload
+; CHECK-NEXT:    add sp, sp, #32
+; CHECK-NEXT:    ret
+  %exn.slot = alloca ptr
+  %ehselector.slot = alloca i32
+  %1 = invoke i32 @foo(i32 42) to label %continue unwind label %cleanup
+
+cleanup:
+  %lp = landingpad { ptr, i32 } cleanup
+  %lp.0 = extractvalue { ptr, i32 } %lp, 0
+  store ptr %lp.0, ptr %exn.slot, align 8
+  %lp.1 = extractvalue { ptr, i32 } %lp, 1
+  store i32 %lp.1, ptr %ehselector.slot, align 4
+  br label %eh.resume
+
+continue:
+  ret void
+
+eh.resume:
+  %exn = load ptr, ptr %exn.slot, align 8
+  unreachable
+}
diff --git a/llvm/unittests/TargetParser/TripleTest.cpp b/llvm/unittests/TargetParser/TripleTest.cpp
index ca48e77cf7caa..36408de7802cc 100644
--- a/llvm/unittests/TargetParser/TripleTest.cpp
+++ b/llvm/unittests/TargetParser/TripleTest.cpp
@@ -2909,6 +2909,10 @@ TEST(TripleTest, DefaultExceptionHandling) {
 
   EXPECT_EQ(ExceptionHandling::DwarfCFI,
             Triple("x86_64-scei-ps4").getDefaultExceptionHandling());
+  EXPECT_EQ(ExceptionHandling::WinEH,
+            Triple("aarch64-pc-windows-msvc").getDefaultExceptionHandling());
+  EXPECT_EQ(ExceptionHandling::DwarfCFI,
+            Triple("aarch64-pc-windows-elf").getDefaultExceptionHandling());
 }
 
 TEST(TripleTest, NormalizeWindows) {

[MaskRay]

5a67ed1 added a x86_64-pc-windows-elf triple test. This looks invalid, probably used a a hack in MCJIT.

[MaskRay]

should state that this is a MCJIT workaround (which means it can retire when MCJIT is removed)

[lhames]

Seems fine to me.

The windows-elf hack lives on in ORC. I think we can kill it off eventually, but we’ll need to improve support for Windows SEH and COFF in general first. We’ll probably need to continue supporting windows-eld for a number of years.

[arsenm]

Merge activity

• Jul 10, 4:04 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jul 10, 4:06 AM UTC: @arsenm merged this pull request with Graphite.