[PATCH] D33436: [ARM] Create relocation for Thumb functions calling ARM fns.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
Herald added subscribers: javed.absar, rengolin, aemerson.

Without using a fixup in this case, BL will be used instead of BLX to
call internal ARM functions from Thumb functions.

It may be possible to change the BL instruction to a BLX instruction at
that point, but I am not aware how this could be done in the AsmBackend.

I had to update the thumb-far-jump.s  test case, because it branches to
a label instead of a function symbol and such symbols are not marked
Thumb functions. This means Asm.isThumbFunc returns false for labels in
such functions, meaning we may produce unnecessary relocations for branches
inside Thumb functions, but we should not miss cases where ARM functions are
called from Thumb functions.

With this patch, multiple runs of the LNT test suite pass built for ARM using
a version of clang that randomly adds +thumb-mode to functions:

index e47b480..da52991 100644

- a/lib/CodeGen/TargetInfo.cpp

+++ b/lib/CodeGen/TargetInfo.cpp
@@ -27,6 +27,8 @@
 #include "llvm/IR/Type.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>    // std::sort
+#include <cstdlib>
+#include <ctime>

using namespace clang;
 using namespace CodeGen;
@@ -5387,7 +5389,9 @@ private:
 class ARMTargetCodeGenInfo : public TargetCodeGenInfo {
public:

  ARMTargetCodeGenInfo(CodeGenTypes &CGT, ARMABIInfo::ABIKind K)

- :TargetCodeGenInfo(new ARMABIInfo(CGT, K)) {}

+    :TargetCodeGenInfo(new ARMABIInfo(CGT, K)) {
+      srand((int)time(0));
+    }

  const ARMABIInfo &getABIInfo() const {
    return static_cast<const ARMABIInfo&>(TargetCodeGenInfo::getABIInfo());

@@ -5421,6 +5425,27 @@ public:

  if (!FD)
    return;

+    llvm::Function *Fn = cast<llvm::Function>(GV);
+
+    int x = rand() % 2;
+    if (x == 0) {
+      llvm::Attribute targetFeatures;
+      if (Fn->hasFnAttribute("target-features")) {
+        targetFeatures = Fn->getFnAttribute("target-features");
+      }
+
+      std::string newFeatures = targetFeatures.getValueAsString();
+      // Do not add thumb-mode if it is already part of the target-features.
+      if (newFeatures.find("thumb-mode") != std::string::npos)
+        return;
+
+      if (newFeatures.size() > 0)
+        newFeatures += ",";
+
+      newFeatures += "+thumb-mode";
+      Fn->addFnAttr("target-features", newFeatures);
+    }
+

  const ARMInterruptAttr *Attr = FD->getAttr<ARMInterruptAttr>();
  if (!Attr)
    return;

@@ -5435,7 +5460,6 @@ public:

  case ARMInterruptAttr::UNDEF:   Kind = "UNDEF"; break;
  }

- llvm::Function *Fn = cast<llvm::Function>(GV);

  Fn->addFnAttr("interrupt", Kind);


https://reviews.llvm.org/D33436

Files:
  lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
  test/MC/ARM/big-endian-thumb-fixup.s
  test/MC/ARM/mixed-arm-thumb-bl-fixup.s
  test/MC/ARM/thumb-far-jump.s


Index: test/MC/ARM/thumb-far-jump.s
===================================================================
--- test/MC/ARM/thumb-far-jump.s
+++ test/MC/ARM/thumb-far-jump.s
@@ -22,5 +22,5 @@
 main2:
 	bx	lr
 
-@ CHECK-NOT: 0x0 R_ARM_THM_CALL end 0x0
+@ CHECK: 0x0 R_ARM_THM_CALL end 0x0
 @ CHECK: 0x2004 R_ARM_THM_CALL main2 0x0
Index: test/MC/ARM/mixed-arm-thumb-bl-fixup.s
===================================================================
--- /dev/null
+++ test/MC/ARM/mixed-arm-thumb-bl-fixup.s
@@ -0,0 +1,47 @@
+@ RUN: llvm-mc < %s -triple armv7-linux-gnueabi -filetype=obj -o - \
+@ RUN:   | llvm-readobj -r | FileCheck %s
+
+	.code	16
+	.thumb_func
+thumb_caller:
+	bl	internal_arm_fn
+	bl	global_arm_fn
+	bl	internal_thumb_fn
+	bl	global_thumb_fn
+
+	.globl	arm_caller
+	.code	32
+arm_caller:
+	bl	internal_arm_fn
+	bl	global_arm_fn
+	bl	internal_thumb_fn
+	bl	global_thumb_fn
+
+	.code	32
+internal_arm_fn:
+	bx	lr
+
+	.globl	global_arm_fn
+	.type	global_arm_fn,%function
+	.code	32
+global_arm_fn:
+	bx	lr
+
+	.code	16
+	.thumb_func
+internal_thumb_fn:
+	bx	lr
+
+	.globl	global_thumb_fn
+	.code	16
+	.thumb_func
+global_thumb_fn:
+	bx	lr
+
+@ CHECK: 0x0 R_ARM_THM_CALL internal_arm_fn 0x0
+@ CHECK: 0x4 R_ARM_THM_CALL global_arm_fn 0x0
+@ CHECK: 0xC R_ARM_THM_CALL global_thumb_fn 0x0
+@ CHECK: 0x10 R_ARM_CALL internal_arm_fn 0x0
+@ CHECK: 0x14 R_ARM_CALL global_arm_fn 0x0
+@ CHECK: 0x18 R_ARM_CALL internal_thumb_fn 0x0
+@ CHECK: 0x1C R_ARM_CALL global_thumb_fn 0x0
Index: test/MC/ARM/big-endian-thumb-fixup.s
===================================================================
--- test/MC/ARM/big-endian-thumb-fixup.s
+++ test/MC/ARM/big-endian-thumb-fixup.s
@@ -4,6 +4,7 @@
 	.text
 	.align	2
 	.code 16
+	.thumb_func
 
 @ARM::fixup_arm_thumb_bl
 .section s_thumb_bl,"ax",%progbits
Index: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
===================================================================
--- lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -729,6 +729,10 @@
     // linker can handle it. GNU AS produces an error in this case.
     if (Sym->isExternal() || Value >= 0x400004)
       IsResolved = false;
+    // When an ARM function is called from a Thumb function, produce a
+    // relocation so the linker will use the correct branch instruction.
+    if (!Asm.isThumbFunc(Sym))
+      IsResolved = false;
   }
   // We must always generate a relocation for BL/BLX instructions if we have
   // a symbol to reference, as the linker relies on knowing the destination


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33436.99879.patch
Type: text/x-patch
Size: 2548 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170523/4151a3b7/attachment.bin>