[GlobalISel][IRTranslator] Emit trap intrinsic for unreachable

[aemerson]

Summary

We were previously just ignoring unreachable, but targets like Darwin want to keep unreachable instructions as traps.

[aemerson]

Unfortunately this regresses code size (as expected) by 0.1% geomean on -Os CTMark:

Program             outputgHVrPT outputLpdj4U diff 
 7zip-benchmark      569484       570496       0.2%
 clamscan            382376       382744       0.1%
 pairlocalalign      248908       249140       0.1%
 consumer-typeset    418972       419224       0.1%
 sqlite3             287088       287236       0.1%
 kc                  432524       432736       0.0%
 bullet              475372       475596       0.0%
 lencod              429576       429764       0.0%
 tramp3d-v4          365880       365916       0.0%
 SPASS               412852       412880       0.0%
 Geomean difference                            0.1%

We'll have to eat this and make up for it elsewhere.

[aemerson]

In D110603#3026589, @aemerson wrote:

Unfortunately this regresses code size (as expected) by 0.1% geomean on -Os CTMark:

Program             outputgHVrPT outputLpdj4U diff 
 7zip-benchmark      569484       570496       0.2%
 clamscan            382376       382744       0.1%
 pairlocalalign      248908       249140       0.1%
 consumer-typeset    418972       419224       0.1%
 sqlite3             287088       287236       0.1%
 kc                  432524       432736       0.0%
 bullet              475372       475596       0.0%
 lencod              429576       429764       0.0%
 tramp3d-v4          365880       365916       0.0%
 SPASS               412852       412880       0.0%
 Geomean difference                            0.1%

We'll have to eat this and make up for it elsewhere.

However, when you add this patch on top of D110610 and D98200 the incremental regression is very minor.

[aemerson]

ping

[paquette]

LGTM

[thakis]

This seems to break tests on mac/arm: http://45.33.8.238/macm1/19172/step_7.txt

(The bot hasn't cycled with https://github.com/llvm/llvm-project/commit/8bde5e58c02c14b43904e9c2e08cca1ab20fafe5 yet, so if that's fixing this, please ignore :) )

[thakis]

Now it has cycled with that change and it didn't help. PTAL :)

[leonardchan]

I believe we're also hitting a similar error at https://luci-milo.appspot.com/p/fuchsia/builders/toolchain.ci/clang-mac-x64/b8834277085583722465:

[504/509] Building CXX object libcxx/src/CMakeFiles/cxx_static.dir/filesystem/operations.cpp.o
FAILED: libcxx/src/CMakeFiles/cxx_static.dir/filesystem/operations.cpp.o 
/opt/s/w/ir/x/w/staging/llvm_install/bin/clang++ --target=arm64-apple-darwin --sysroot=/opt/s/w/ir/cache/macos_sdk/XCode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk -DNDEBUG -D_LIBCPP_BUILDING_LIBRARY -D_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilibcxx/include/c++build -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -isysroot /opt/s/w/ir/cache/macos_sdk/XCode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk -mmacosx-version-min=10.15 -fPIC -DLIBCXX_BUILDING_LIBCXXABI -faligned-allocation -nostdinc++ -fvisibility-inlines-hidden -fvisibility=hidden -Wall -Wextra -W -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type -Wextra-semi -Wundef -Wno-user-defined-literals -Wno-covered-switch-default -Wno-suggest-override -Wno-error -I/opt/s/w/ir/x/w/staging/runtimes_build/include/c++/v1 -std=c++20 -MD -MT libcxx/src/CMakeFiles/cxx_static.dir/filesystem/operations.cpp.o -MF libcxx/src/CMakeFiles/cxx_static.dir/filesystem/operations.cpp.o.d -o libcxx/src/CMakeFiles/cxx_static.dir/filesystem/operations.cpp.o -c /opt/s/w/ir/x/w/llvm-project/libcxx/src/filesystem/operations.cpp
unknown operand type
UNREACHABLE executed at llvm/lib/Target/AArch64/AArch64MCInstLower.cpp:262!
clang-14: error: clang frontend command failed with exit code 134 (use -v to see invocation)
Fuchsia clang version 14.0.0 (https://llvm.googlesource.com/a/llvm-project e8477045f6d8229a60f6d10c984ee84a3b05efdf)
Target: arm64-apple-darwin
Thread model: posix
InstalledDir: /opt/s/w/ir/x/w/staging/llvm_install/bin
clang-14: note: diagnostic msg:

[aemerson]

In D110603#3041058, @leonardchan wrote:

I believe we're also hitting a similar error at https://luci-milo.appspot.com/p/fuchsia/builders/toolchain.ci/clang-mac-x64/b8834277085583722465:

[504/509] Building CXX object libcxx/src/CMakeFiles/cxx_static.dir/filesystem/operations.cpp.o
FAILED: libcxx/src/CMakeFiles/cxx_static.dir/filesystem/operations.cpp.o 
/opt/s/w/ir/x/w/staging/llvm_install/bin/clang++ --target=arm64-apple-darwin --sysroot=/opt/s/w/ir/cache/macos_sdk/XCode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk -DNDEBUG -D_LIBCPP_BUILDING_LIBRARY -D_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilibcxx/include/c++build -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -isysroot /opt/s/w/ir/cache/macos_sdk/XCode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk -mmacosx-version-min=10.15 -fPIC -DLIBCXX_BUILDING_LIBCXXABI -faligned-allocation -nostdinc++ -fvisibility-inlines-hidden -fvisibility=hidden -Wall -Wextra -W -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type -Wextra-semi -Wundef -Wno-user-defined-literals -Wno-covered-switch-default -Wno-suggest-override -Wno-error -I/opt/s/w/ir/x/w/staging/runtimes_build/include/c++/v1 -std=c++20 -MD -MT libcxx/src/CMakeFiles/cxx_static.dir/filesystem/operations.cpp.o -MF libcxx/src/CMakeFiles/cxx_static.dir/filesystem/operations.cpp.o.d -o libcxx/src/CMakeFiles/cxx_static.dir/filesystem/operations.cpp.o -c /opt/s/w/ir/x/w/llvm-project/libcxx/src/filesystem/operations.cpp
unknown operand type
UNREACHABLE executed at llvm/lib/Target/AArch64/AArch64MCInstLower.cpp:262!
clang-14: error: clang frontend command failed with exit code 134 (use -v to see invocation)
Fuchsia clang version 14.0.0 (https://llvm.googlesource.com/a/llvm-project e8477045f6d8229a60f6d10c984ee84a3b05efdf)
Target: arm64-apple-darwin
Thread model: posix
InstalledDir: /opt/s/w/ir/x/w/staging/llvm_install/bin
clang-14: note: diagnostic msg:

Sorry about that. I'll revert while I investigate.

[uabelho]

Hi,

I think there is something broken after the relanding of this.
For my out-of-tree target I have a testcase where there exists a block that is unreachable from entry, but that is referenced in a PHI:

body: |
  bb.0:
    liveins: $a0h, $r0
    %0:all(s16) = COPY $a0h
    %1:all(s16) = COPY $r0
    brr_uncond %bb.2

  bb.1:
    brr_uncond %bb.2

  bb.2:
    %2:all(s16) = G_PHI %0(s16), %bb.0, %1(s16), %bb.1
    $a0h = COPY %2

So %bb.1 is dead, but jumps to %bb.2 so the G_PHI in %bb.2 references %bb.1

With this patch we get this result:

body:             |
  ; Instruction count: 5
  bb.0:
    successors: %bb.2(0x80000000)
    liveins: $a0h, $r0
  
    %0:anh_rn = COPY $a0h
    %1:anh_rn = COPY $r0
    brr_uncond %bb.2
  
  bb.2:
    %2:anh_rn = PHI %0, %bb.0, %1, %bb.-1
    $a0h = COPY %2

So %bb.1 is removed, but the PHI now looks broken because %bb.1 has been replace with %bb.-1 instead of the operand being removed completely.

[aemerson]

In D110603#3041995, @uabelho wrote:

Hi,

I think there is something broken after the relanding of this.
For my out-of-tree target I have a testcase where there exists a block that is unreachable from entry, but that is referenced in a PHI:

body: |
  bb.0:
    liveins: $a0h, $r0
    %0:all(s16) = COPY $a0h
    %1:all(s16) = COPY $r0
    brr_uncond %bb.2

  bb.1:
    brr_uncond %bb.2

  bb.2:
    %2:all(s16) = G_PHI %0(s16), %bb.0, %1(s16), %bb.1
    $a0h = COPY %2

So %bb.1 is dead, but jumps to %bb.2 so the G_PHI in %bb.2 references %bb.1

With this patch we get this result:

body:             |
  ; Instruction count: 5
  bb.0:
    successors: %bb.2(0x80000000)
    liveins: $a0h, $r0
  
    %0:anh_rn = COPY $a0h
    %1:anh_rn = COPY $r0
    brr_uncond %bb.2
  
  bb.2:
    %2:anh_rn = PHI %0, %bb.0, %1, %bb.-1
    $a0h = COPY %2

So %bb.1 is removed, but the PHI now looks broken because %bb.1 has been replace with %bb.-1 instead of the operand being removed completely.

That's unfortunate. It looks to me that the MIR is invalid. At the IR level simplifycfg should have deleted those blocks and fixed up any phis.

I'll split the fix for this into a separate patch for review.

llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h

bool translateFPToSI(const User &U, MachineIRBuilder &MIRBuilder) {
     return translateCast(TargetOpcode::G_FPTOSI, U, MIRBuilder);
   }
   bool translateUIToFP(const User &U, MachineIRBuilder &MIRBuilder) {
     return translateCast(TargetOpcode::G_UITOFP, U, MIRBuilder);
   }
   bool translateSIToFP(const User &U, MachineIRBuilder &MIRBuilder) {
     return translateCast(TargetOpcode::G_SITOFP, U, MIRBuilder);
   }
-  bool translateUnreachable(const User &U, MachineIRBuilder &MIRBuilder) {
-    return true;
-  }
+  bool translateUnreachable(const User &U, MachineIRBuilder &MIRBuilder);
+
   bool translateSExt(const User &U, MachineIRBuilder &MIRBuilder) {
     return translateCast(TargetOpcode::G_SEXT, U, MIRBuilder);
   }
 
   bool translateZExt(const User &U, MachineIRBuilder &MIRBuilder) {
     return translateCast(TargetOpcode::G_ZEXT, U, MIRBuilder);
   }
 

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

bool IRTranslator::translateVAArg(const User &U, MachineIRBuilder &MIRBuilder) {
   // others). Fortunately the ABIs I know of where that matters don't use va_arg
   // anyway but that's not guaranteed.
   MIRBuilder.buildInstr(TargetOpcode::G_VAARG, {getOrCreateVReg(U)},
                         {getOrCreateVReg(*U.getOperand(0)),
                          DL->getABITypeAlign(U.getType()).value()});
   return true;
 }
 
+bool IRTranslator::translateUnreachable(const User &U, MachineIRBuilder &MIRBuilder) {
+    if (!MF->getTarget().Options.TrapUnreachable)
+    return true;
+
+  auto &UI = cast<UnreachableInst>(U);
+  // We may be able to ignore unreachable behind a noreturn call.
+  if (MF->getTarget().Options.NoTrapAfterNoreturn) {
+    const BasicBlock &BB = *UI.getParent();
+    if (&UI != &BB.front()) {
+      BasicBlock::const_iterator PredI =
+        std::prev(BasicBlock::const_iterator(UI));
+      if (const CallInst *Call = dyn_cast<CallInst>(&*PredI)) {
+        if (Call->doesNotReturn())
+          return true;
+      }
+    }
+  }
+
+  MIRBuilder.buildIntrinsic(Intrinsic::trap, ArrayRef<Register>(), true);
+  return true;
+}
+
 bool IRTranslator::translateInsertElement(const User &U,
                                           MachineIRBuilder &MIRBuilder) {
   // If it is a <1 x Ty> vector, use the scalar as it is
   // not a legal vector type in LLT.
   if (cast<FixedVectorType>(U.getType())->getNumElements() == 1)
     return translateCopy(U, *U.getOperand(1), MIRBuilder);
 
   Register Res = getOrCreateVReg(U);
   Register Val = getOrCreateVReg(*U.getOperand(0));
   Register Elt = getOrCreateVReg(*U.getOperand(1));

llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-unreachable.ll

+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+; RUN: llc -O0 -mtriple=aarch64-apple-ios -global-isel -stop-after=irtranslator %s -o - | FileCheck %s
+
+declare void @llvm.trap()
+
+define void @unreachable() {
+  ; CHECK-LABEL: name: unreachable
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK-NEXT:   G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.trap)
+  unreachable
+  ret void
+}
+
+declare void @foo() noreturn
+define void @trap_call_noreturn() {
+  ; CHECK-LABEL: name: trap_call_noreturn
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   BL @foo, csr_darwin_aarch64_aapcs, implicit-def $lr, implicit $sp
+  ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+  call void @foo()
+  unreachable
+  ret void
+}