Enable EHABI by default on non-Darwin ARM targets

[rengolin]

Summary

After all hard work to implement the EHABI and with the test-suite passing, it's time to turn it on by default and allow users to disable it as a work-around while we fix the eventual bugs that show up.

This commit also remove the -arm-enable-ehabi-descriptors, since we want the tables to be printed every time the EHABI is turned on for non-Darwin ARM targets.

Current EH test in the test-suite pass with these changes, so a follow-
up commit on the test-suite will re-enable them.

The only problem is that MCJIT can't handle some new relocation, so would be good to disable EHABI while JIT-ing, but I have no idea how. Most (all?) MCJIT tests fail with this commit, so we need to either disable that or fix it. I'd rather disable to get to work on it in separate, since static compilation is working well.

Not implemented relocation type!
UNREACHABLE executed at /home/user/devel/llvm/src/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:482!

[asl]

Maybe we temporary keep the option, but make it enabled by default? So MCJIT users can switch it off until missed relocs support will be added?

[rengolin]

Hi Anton,

I kept the option as "-arm-disable-ehabi", but requiring all MCJIT users to start using them would be cheeky. Meanwhile, I'll have a look at which relocation is missing and try to implement it, but that might not be the only problem and I could spend a huge time chasing the wrong problem.

The main issue with the MCAsmInfo is that there is no way to turn it on/off dynamically (only based on that flag and being ELF), so another possibility would be to add a dynamic disableEH() and call it from MCJIT's drivers. I'll do this one first and add to this commit and chase the relocation later. We should be able to remove this method once everything works fine for a while.

[rengolin]

I traced back the problems on MCJIT:

1. two relocations needed recognizing (NONE, PREL31), which could easily be implemented by ignoring NONE and matching PREL31 with existing similar relocations (I need to make sure this is right)
2. tests failing should have nounwind on them, since there's nothing about EH on any of them. We should make sure those functions fall into "if (!Asm->MF->getFunction()->needsUnwindTableEntry())" case. I'll work on that now.
3. '__aeabi_unwind_cpp_pr0' which could not be resolved. When JITing C++ code with exceptions, we need to make sure the right libraries are also loaded. I have no idea how to make that.

In theory, just fixing the problem 1 and 2 above should make the tests pass, but if we add C++/EH tests, we need to make sure the libs are loaded.

[rengolin]

Right, MCJIT crisis averted, all tests and the test-suite passing, I think it's good to go.

[asl]

Otherwise - LGTM. Thanks for finalizing this! :)

lib/Target/ARM/ARMAsmPrinter.cpp

1125–1126

Maybe we'd explicitly check for EABI subtarget here?

[rengolin]

lib/Target/ARM/ARMAsmPrinter.cpp

1125–1126

That's a good point.

I couldn't find any flag that would directly translate to EABI compatible on all systems (including gnueabi, android, freebsd), but I think this would do:

bool isEHABICompatible = Subtarget->isTargetAEABI() ||
                         Subtarget->isTargetELF() ||
                         Subtarget->isTargetLinux();
if (isEHABICompatible && !DisableARMEHABI &&
     MI->getFlag(MachineInstr::FrameSetup))
  EmitUnwindingInstruction(MI);

[asl]

lib/Target/ARM/ARMAsmPrinter.cpp

1125–1126

Hrm, should we really emit unwinding stuff for ELF targets w/o AEABI? And is it possible that isTargetAEABI() will be false and isLinux() will be true?

[rengolin]

lib/Target/ARM/ARMAsmPrinter.cpp

1125–1126

Yes, for gnueabi, that's the norm. AEABI is only enabled with the target is *explicitly* "EABI".

I'm fine with removing the ELF flag, since that was just a guess, but Linux generally means gnueabi, and in fact, if the triple is gnueabi, that won't generate the tables.

I could also add a Subtarget->hasEHABISupport() and do some fiddling with the triples...

[rengolin]

Adding isTargetEHABICompatible() to ARMSubtarget, so that we can have a precise definition of an EHABI compliant target. Other targets that also support should change this method accordingly.

[rengolin]

Hi Anton,

I needed to get the test-suite bot running a few iterations before the end of the week, so I committed (r200388), we can make the new function more specific later, for now it's good enough. Thanks for the review!

lib/CodeGen/AsmPrinter/ARMException.cpp

 #include "llvm/Target/TargetRegisterInfo.h"
 using namespace llvm;
 
-static cl::opt<bool>
-EnableARMEHABIDescriptors("arm-enable-ehabi-descriptors", cl::Hidden,
-  cl::desc("Generate ARM EHABI tables with unwinding descriptors"),
-  cl::init(false));
-
-
 ARMException::ARMException(AsmPrinter *A)
   : DwarfException(A) {}
 

     Asm->OutStreamer.EmitLabel(Asm->GetTempSymbol("eh_func_end",
                                                   Asm->getFunctionNumber()));
 
-    if (EnableARMEHABIDescriptors) {
-      // Map all labels and get rid of any dead landing pads.
-      MMI->TidyLandingPads();
+    // Map all labels and get rid of any dead landing pads.
+    MMI->TidyLandingPads();
 
-      if (!MMI->getLandingPads().empty()) {
-        // Emit references to personality.
-        if (const Function * Personality =
-            MMI->getPersonalities()[MMI->getPersonalityIndex()]) {
-          MCSymbol *PerSym = Asm->getSymbol(Personality);
-          Asm->OutStreamer.EmitSymbolAttribute(PerSym, MCSA_Global);
-          ATS.emitPersonality(PerSym);
-        }
 
-        // Emit .handlerdata directive.
-        ATS.emitHandlerData();
 
-        // Emit actual exception table
-        EmitExceptionTable();
-      }
     }
   }
 

lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp

   default:
     llvm_unreachable("Not implemented relocation type!");
 
+  case ELF::R_ARM_NONE:
+    break;
   // Write a 32bit value to relocation address, taking into account the
   // implicit addend encoded in the target.
+  case ELF::R_ARM_PREL31:
   case ELF::R_ARM_TARGET1:
   case ELF::R_ARM_ABS32:
     *TargetPtr = *Placeholder + Value;

lib/Target/ARM/ARMAsmPrinter.cpp

   }
 }
 
-extern cl::opt<bool> EnableARMEHABI;
+extern cl::opt<bool> DisableARMEHABI;
 
 // Simple pseudo-instructions have their lowering (with expansion to real
 // instructions) auto-generated.

   }
 
   // Emit unwinding stuff for frame-related instructions
-  if (EnableARMEHABI && MI->getFlag(MachineInstr::FrameSetup))
+  if (Subtarget->isTargetEHABICompatible() && !DisableARMEHABI &&
+       MI->getFlag(MachineInstr::FrameSetup))
     EmitUnwindingInstruction(MI);
 
   // Do any auto-generated pseudo lowerings.

lib/Target/ARM/ARMSubtarget.h

            !isTargetDarwin();
   }
 
+  // ARM Targets that support EHABI exception handling standard
+  // Darwin uses SjLj. Other targets might need more checks.
+  bool isTargetEHABICompatible() const {
+    return (TargetTriple.getEnvironment() == Triple::EABI ||
+            TargetTriple.getEnvironment() == Triple::GNUEABI ||
+            TargetTriple.getEnvironment() == Triple::EABIHF ||
+            TargetTriple.getEnvironment() == Triple::GNUEABIHF) &&
+           !isTargetDarwin();
+  }
+
   bool isTargetHardFloat() const {
     return TargetTriple.getEnvironment() == Triple::GNUEABIHF ||
            TargetTriple.getEnvironment() == Triple::EABIHF;

lib/Target/ARM/MCTargetDesc/ARMMCAsmInfo.cpp

 
 using namespace llvm;
 
+// ARM EHABI is experimental but the quality is good enough
+// to be turned on by default on non-Darwin ARM targets.
 cl::opt<bool>
-EnableARMEHABI("arm-enable-ehabi", cl::Hidden,
-  cl::desc("Generate ARM EHABI tables"),
+DisableARMEHABI("arm-disable-ehabi", cl::Hidden,
+  cl::desc("Disable ARM experimental exception handling"),
   cl::init(false));
 
 

   SupportsDebugInformation = true;
 
   // Exceptions handling
-  if (EnableARMEHABI)
+  if (!DisableARMEHABI)
     ExceptionsType = ExceptionHandling::ARM;
 
   // foo(plt) instead of foo@plt

test/CodeGen/ARM/arm-ttype-target2.ll

-; RUN: llc -mtriple=armv7-none-linux-gnueabi -arm-enable-ehabi -arm-enable-ehabi-descriptors < %s | FileCheck %s 
+; RUN: llc -mtriple=armv7-none-linux-gnueabi < %s | FileCheck %s
 
 @_ZTVN10__cxxabiv117__class_type_infoE = external global i8*
 @_ZTS3Foo = linkonce_odr constant [5 x i8] c"3Foo\00"

test/CodeGen/ARM/ehabi-filters.ll

-; RUN: llc -arm-enable-ehabi -arm-enable-ehabi-descriptors < %s | FileCheck %s
+; RUN: llc < %s | FileCheck %s
 target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:64:128-a0:0:64-n32-S64"
 target triple = "armv7-none-linux-gnueabi"
 

test/CodeGen/ARM/ehabi-no-landingpad.ll

-; RUN: llc < %s -mtriple=armv7-unknown-linux-gnueabi \
-; RUN:   -arm-enable-ehabi -arm-enable-ehabi-descriptors | FileCheck %s
+; RUN: llc < %s -mtriple=armv7-unknown-linux-gnueabi | FileCheck %s
 
 target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:64:128-a0:0:64-n32-S64"
 target triple = "armv7-unknown-linux-gnueabi"

test/CodeGen/ARM/ehabi-unwind.ll

 ; Test that the EHABI unwind instruction generator does not encounter any
 ; unfamiliar instructions.
-; RUN: llc < %s -mtriple=thumbv7 -arm-enable-ehabi -disable-fp-elim
-; RUN: llc < %s -mtriple=thumbv7 -arm-enable-ehabi
-; RUN: llc < %s -mtriple=thumbv7 -arm-enable-ehabi -arm-enable-ehabi-descriptors
+; RUN: llc < %s -mtriple=thumbv7 -disable-fp-elim
+; RUN: llc < %s -mtriple=thumbv7
 
 define void @_Z1fv() nounwind {
 entry:

test/CodeGen/ARM/ehabi.ll

 ; (4) armv7 without -disable-fp-elim
 
 ; RUN: llc -mtriple arm-unknown-linux-gnueabi \
-; RUN:     -arm-enable-ehabi -arm-enable-ehabi-descriptors \
 ; RUN:     -disable-fp-elim -filetype=asm -o - %s \
 ; RUN:   | FileCheck %s --check-prefix=CHECK-FP
 
 ; RUN: llc -mtriple arm-unknown-linux-gnueabi \
-; RUN:     -arm-enable-ehabi -arm-enable-ehabi-descriptors \
 ; RUN:     -filetype=asm -o - %s \
 ; RUN:   | FileCheck %s --check-prefix=CHECK-FP-ELIM
 
 ; RUN: llc -mtriple armv7-unknown-linux-gnueabi \
-; RUN:     -arm-enable-ehabi -arm-enable-ehabi-descriptors \
 ; RUN:     -disable-fp-elim -filetype=asm -o - %s \
 ; RUN:   | FileCheck %s --check-prefix=CHECK-V7-FP
 
 ; RUN: llc -mtriple armv7-unknown-linux-gnueabi \
-; RUN:     -arm-enable-ehabi -arm-enable-ehabi-descriptors \
 ; RUN:     -filetype=asm -o - %s \
 ; RUN:   | FileCheck %s --check-prefix=CHECK-V7-FP-ELIM
 

test/CodeGen/ARM/setcc-sentinals.ll

-; RUN: llc < %s -mcpu=cortex-a8 -march=arm -asm-verbose=false | FileCheck %s
+; RUN: llc < %s -mcpu=cortex-a8 -march=arm -asm-verbose=false -arm-disable-ehabi | FileCheck %s
 
 define zeroext i1 @test0(i32 %x) nounwind {
 ; CHECK-LABEL: test0:

test/CodeGen/Thumb2/constant-islands.ll

-; RUN: llc < %s -march=arm   -mcpu=cortex-a8 -O0 -filetype=obj -o %t.o
-; RUN: llc < %s -march=thumb -mcpu=cortex-a8 -O0 -filetype=obj -o %t.o
-; RUN: llc < %s -march=arm   -mcpu=cortex-a8 -O2 -filetype=obj -verify-machineinstrs -o %t.o
-; RUN: llc < %s -march=thumb -mcpu=cortex-a8 -O2 -filetype=obj -verify-machineinstrs -o %t.o
+; RUN: llc < %s -march=arm   -mcpu=cortex-a8 -O0 -filetype=obj -o %t.o -arm-disable-ehabi
+; RUN: llc < %s -march=thumb -mcpu=cortex-a8 -O0 -filetype=obj -o %t.o -arm-disable-ehabi
+; RUN: llc < %s -march=arm   -mcpu=cortex-a8 -O2 -filetype=obj -verify-machineinstrs -o %t.o -arm-disable-ehabi
+; RUN: llc < %s -march=thumb -mcpu=cortex-a8 -O2 -filetype=obj -verify-machineinstrs -o %t.o -arm-disable-ehabi
 target datalayout = "e-p:32:32:32-i1:8:32-i8:8:32-i16:16:32-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:32:64-v128:32:128-a0:0:32-n32-S32"
 target triple = "thumbv7-apple-ios"
 

test/ExecutionEngine/MCJIT/remote/Inputs/cross-module-b.ll

 declare i32 @FA()
 
-define i32 @FB() {
+define i32 @FB() nounwind {
   %r = call i32 @FA( )   ; <i32> [#uses=1]
   ret i32 %r
 }

test/ExecutionEngine/MCJIT/remote/Inputs/multi-module-b.ll

 declare i32 @FC()
 
-define i32 @FB() {
+define i32 @FB() nounwind {
   %r = call i32 @FC( )   ; <i32> [#uses=1]
   ret i32 %r
 }

test/ExecutionEngine/MCJIT/remote/Inputs/multi-module-c.ll

-define i32 @FC() {
+define i32 @FC() nounwind {
   ret i32 0
 }
 

test/ExecutionEngine/MCJIT/remote/cross-module-a.ll

 
 declare i32 @FB()
 
-define i32 @FA() {
+define i32 @FA() nounwind {
   ret i32 0
 }
 
-define i32 @main() {
+define i32 @main() nounwind {
   %r = call i32 @FB( )   ; <i32> [#uses=1]
   ret i32 %r
 }
-

test/ExecutionEngine/MCJIT/remote/multi-module-a.ll

 
 declare i32 @FB()
 
-define i32 @main() {
+define i32 @main() nounwind {
   %r = call i32 @FB( )   ; <i32> [#uses=1]
   ret i32 %r
 }

test/ExecutionEngine/MCJIT/remote/simpletest-remote.ll

 ; RUN: %lli_mcjit -remote-mcjit -mcjit-remote-process=lli-child-target%exeext %s > /dev/null
 
-define i32 @bar() {
+define i32 @bar() nounwind {
   ret i32 0
 }
 
-define i32 @main() {
+define i32 @main() nounwind {
   %r = call i32 @bar( )   ; <i32> [#uses=1]
   ret i32 %r
 }

test/ExecutionEngine/MCJIT/remote/test-data-align-remote.ll

 ; Check that a variable is always aligned as specified.
 
 @var = global i32 0, align 32
-define i32 @main() {
+define i32 @main() nounwind {
   %addr = ptrtoint i32* @var to i64
   %mask = and i64 %addr, 31
   %tst = icmp eq i64 %mask, 0

test/ExecutionEngine/MCJIT/remote/test-fp-no-external-funcs-remote.ll

 ; RUN: %lli_mcjit -remote-mcjit -mcjit-remote-process=lli-child-target%exeext %s > /dev/null
 
-define double @test(double* %DP, double %Arg) {
+define double @test(double* %DP, double %Arg) nounwind {
   %D = load double* %DP   ; <double> [#uses=1]
   %V = fadd double %D, 1.000000e+00   ; <double> [#uses=2]
   %W = fsub double %V, %V   ; <double> [#uses=3]

   ret double %Y
 }
 
-define i32 @main() {
+define i32 @main() nounwind {
   %X = alloca double    ; <double*> [#uses=2]
   store double 0.000000e+00, double* %X
   call double @test( double* %X, double 2.000000e+00 )    ; <double>:1 [#uses=0]

test/ExecutionEngine/MCJIT/remote/test-global-init-nonzero-remote.ll

 
 @count = global i32 1, align 4
 
-define i32 @main() nounwind uwtable {
+define i32 @main() nounwind {
 entry:
   %retval = alloca i32, align 4
   %i = alloca i32, align 4

test/MC/ARM/data-in-code.ll

-;; RUN: llc -O0 -verify-machineinstrs -fast-isel-abort \
+;; RUN: llc -O0 -verify-machineinstrs -fast-isel-abort -arm-disable-ehabi \
 ;; RUN:   -mtriple=armv7-linux-gnueabi -filetype=obj %s -o - | \
 ;; RUN:   llvm-readobj -t | FileCheck -check-prefix=ARM %s
 
-;; RUN: llc -O0 -verify-machineinstrs -fast-isel-abort \
+;; RUN: llc -O0 -verify-machineinstrs -fast-isel-abort -arm-disable-ehabi \
 ;; RUN:   -mtriple=thumbv7-linux-gnueabi -filetype=obj %s -o - | \
 ;; RUN:   llvm-readobj -t | FileCheck -check-prefix=TMB %s
 

test/MC/ARM/elf-thumbfunc-reloc.ll

 ; CHECK-NEXT:   Section (2) .rel.text {
 ; CHECK-NEXT:     0x8 R_ARM_THM_CALL foo 0x0
 ; CHECK-NEXT:   }
+; CHECK-NEXT:   Section (7) .rel.ARM.exidx {
+; CHECK-NEXT:     0x0 R_ARM_PREL31 .text 0x0
+; CHECK-NEXT:     0x8 R_ARM_PREL31 .text 0x0
+; CHECK-NEXT:   }
 ; CHECK-NEXT: ]
 
 ; make sure foo is thumb function: bit 0 = 1