[ARM] Support -mexecute-only with -mlong-calls.

[ZhiyaoMa98]

Summary

Instead of using constant pools, use movw movt pair.

[efriedma]

llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll

19

Is there some reason we can't just generate movw r0, :lower16:bar; movt r0, :upper16:bar?

[ZhiyaoMa98]

@efriedma Thank you for your suggestion. I will remove the extra indirection.

I was wondering if you could also provide some insights about the RWPI case. I believe the same optimization also applies to RWPI. However, I actually want to store the function address as a global variable when using RWPI, because I want the address to live in RAM instead of Flash, so that I can redirect the function call at runtime, for dynamic linking purpose.

Should I create a new target feature to indicate that I want to store function address in RAM?

[efriedma]

that I can redirect the function call at runtime, for dynamic linking purpose.

Can you describe a little more what you're trying to do here?

If you want to replace the implementation of an existing function at runtime, you'd be better off implementing the indirection as a frontend feature; by the time you get to the backend, optimizations have destroyed the semantics you want.

[ZhiyaoMa98]

Can you describe a little more what you're trying to do here?

Sure. My eventual goal is to enable fine-granular live-update on ARM based microcontrollers, which requires the system to do some relocation at runtime. Below I will describe the challenge with a simple C example.

Consider the following C snippet:

extern void global_func(void); // A global function whose symbol is exported by the system at runtime.
static void local_func(void) { ... }

static void main_entry(void) {
    local_func();
    global_func();
}

I want to load and run the compiled object file at runtime, which requires two steps.

1. Burn the object file into Flash storage.
2. Perform a runtime symbol resolution and relocation so that global_func is set to the runtime address.

The reason why I must store code in Flash storage is that the microcontroller I am using, as well as many other ARM based microcontrollers, has Flash storage 5x greater than RAM, and code typically directly runs from Flash.

local_func requires the compiler to use position independent code, which has already been handled by -fropi. global_func however, is the case I am trying to solve here.

Existing compiler options always store the address of global_func in Flash.

The default case:

main_entry:
    bl  local_func
    b.w global_func // Relative address is hardcoded in the instruction, in Flash.

If compile with -mlong-calls:

main_entry:
    bl  local_func
    ldr r0, [pc, #4] // Load address from constant pool, still in Flash.
    bx  r0
.Lconst_pool:
    .word global_func

In the hypothetical case if the compiler chose to use movw movt pair:

main_entry:
    bl   local_func
    movw r0, :lower16:global_func // Absolute address is hardcoded in the instruction, still in Flash.
    movt r0, :upper16:global_func
    bx.  r0

I was expecting to use the "side effect" of -mexecute-only that promotes constant pools to global variables to achieve my goal of having the function address to live in RAM.

main_entry:
    bl   local_func
    movw r0, :lower16:.const_pool(sbrel)
    movt r0, :upper16:.const_pool(sbrel) // Also using RWPI so that the jump table can be placed anywhere in RAM pointed by r9.
    ldr  r0, [r9, r0] // Absolute address is held in RAM now.
    bx   r0

As you have already pointed out, in the normal case when we do not need to put the address in RAM, the extra indirection is unnecessary and slows down the code.

But if I have a use case like above where I need to store the address in RAM, could you enlighten me about the best approach to achieve my goal?

[efriedma]

The construct you want is pretty similar to a GOT. if you compile with -fPIE -fsemantic-interposition, you get basically the code you want, except that the compiler uses a plt by default instead of a got. If we supported -fno-plt for ARM, it would be almost exactly what you want. That said, that won't work with -frwpi... maybe we need some new kind of relocation to represent that.

[ZhiyaoMa98]

Unfortunately, -fPIE seems not to be generating the PLT on LLVM for embedded ARM.

C source file (test.c):

extern void bar(void);
void foo(void) {
    bar();
}

LLVM with clang -O2 -fPIE -fsemantic-interposition -mlong-calls --target=armv7em-none-eabi -c test.c:

00000000 <foo>:
   0:   4800            ldr     r0, [pc, #0]    ; (4 <foo+0x4>)
   2:   4700            bx      r0
   4:   00000000        .word   0x00000000

ARM GNU with arm-none-eabi-gcc -O2 -fPIE -mlong-calls -msingle-pic-base -mcpu=cortex-m4 -c test.c:

00000000 <foo>:
   0:   4b01            ldr     r3, [pc, #4]    ; (8 <foo+0x8>)
   2:   f859 3003       ldr.w   r3, [r9, r3]
   6:   4718            bx      r3
   8:   00000000        .word   0x00000000

[efriedma]

One, -mlong-calls isn't currently compatible with PIE. Two, on ARM, there are no special plt relocations; the linker just takes care of it. (You can see the differences if you try to take the address of a function without calling it.)

[ZhiyaoMa98]

I have updated the diff to avoid the extra indirection. I am thinking about adding a new option, say -mgot-calls to allow code generation with the extra indirection. Is it sensible and shall I create another diff to discuss that?

[efriedma]

I am thinking about adding a new option, say -mgot-calls to allow code generation with the extra indirection. Is it sensible and shall I create another diff to discuss that?

That probably makes sense, yes.

llvm/lib/Target/ARM/ARMISelLowering.cpp

2686

Can we directly check that movw/movt is available? I think that's what we do in other places? (Then just assert we aren't execute-only in the non-movw path.)

[ZhiyaoMa98]

Then just assert we aren't execute-only in the non-movw path.

When we are not execute-only, existing code handles it by using constant pools and we are all good.

In the case where we are execute-only and long-calls at the same time, we assert that we have movt like in other places in the same source file.

[efriedma]

LGTM with one small change.

clang/lib/Driver/ToolChains/Arch/ARM.cpp

779

Fix this comment?

[ZhiyaoMa98]

Updated the comment to reflect that now we allow using -mlong-calls with -mexecute-only.

[ZhiyaoMa98]

Remove the unused GA variable.

[ZhiyaoMa98]

Just in case you assume that I have push permission, unfortunately I do not. Could you help me merge the patch in? Thanks.

clang/lib/Driver/ToolChains/Arch/ARM.cpp

if (A->getOption().matches(options::OPT_mlong_calls))
              !Triple.isWatchOS()) {
       Features.push_back("+long-calls");
   }
 
   // Generate execute-only output (no data access to code sections).
   // This only makes sense for the compiler, not for the assembler.
   if (!ForAS) {
     // Supported only on ARMv6T2 and ARMv7 and above.
     // Cannot be combined with -mno-movt or -mlong-calls
     if (Arg *A = Args.getLastArg(options::OPT_mexecute_only, options::OPT_mno_execute_only)) {
       if (A->getOption().matches(options::OPT_mexecute_only)) {
         if (getARMSubArchVersionNumber(Triple) < 7 &&
             llvm::ARM::parseArch(Triple.getArchName()) != llvm::ARM::ArchKind::ARMV6T2)
               D.Diag(diag::err_target_unsupported_execute_only) << Triple.getArchName();
         else if (Arg *B = Args.getLastArg(options::OPT_mno_movt))
-          D.Diag(diag::err_opt_not_valid_with_opt) << A->getAsString(Args) << B->getAsString(Args);
-        // Long calls create constant pool entries and have not yet been fixed up
-        // to play nicely with execute-only. Hence, they cannot be used in
-        // execute-only code for now
-        else if (Arg *B = Args.getLastArg(options::OPT_mlong_calls, options::OPT_mno_long_calls)) {
-          if (B->getOption().matches(options::OPT_mlong_calls))
-            D.Diag(diag::err_opt_not_valid_with_opt) << A->getAsString(Args) << B->getAsString(Args);
-        }
+          D.Diag(diag::err_opt_not_valid_with_opt)
+              << A->getAsString(Args) << B->getAsString(Args);
         Features.push_back("+execute-only");
       }
     }
   }
 
   // Kernel code has more strict alignment requirements.
   if (KernelOrKext) {
     Features.push_back("+strict-align");

clang/test/Driver/arm-execute-only.c

 // RUN: not %clang -c -target thumbv6m-eabi -mexecute-only %s 2>&1 | \
 // RUN:   FileCheck --check-prefix CHECK-EXECUTE-ONLY-NOT-SUPPORTED %s
 // CHECK-EXECUTE-ONLY-NOT-SUPPORTED: error: execute only is not supported for the thumbv6m sub-architecture
 
 // RUN: not %clang -target armv8m.main-eabi -mexecute-only -mno-movt %s 2>&1 \
 // RUN:    | FileCheck %s -check-prefix CHECK-EXECUTE-ONLY-NO-MOVT
 // CHECK-EXECUTE-ONLY-NO-MOVT: error: option '-mexecute-only' cannot be specified with '-mno-movt'
 
-// RUN: not %clang -target armv8m.main-eabi -mexecute-only -mlong-calls %s 2>&1 \
-// RUN:    | FileCheck %s -check-prefix CHECK-EXECUTE-ONLY-LONG-CALLS
-// CHECK-EXECUTE-ONLY-LONG-CALLS: error: option '-mexecute-only' cannot be specified with '-mlong-calls'
-
 // RUN: %clang -target armv7m-eabi -x assembler -mexecute-only %s -c -### 2>&1 \
 // RUN:    | FileCheck %s --check-prefix=CHECK-NO-EXECUTE-ONLY-ASM
 // CHECK-NO-EXECUTE-ONLY-ASM: warning: argument unused during compilation: '-mexecute-only'
 
 // -mpure-code flag for GCC compatibility
 // RUN: not %clang -c -target thumbv6m-eabi -mpure-code %s 2>&1 | \
 // RUN:   FileCheck --check-prefix CHECK-EXECUTE-ONLY-NOT-SUPPORTED %s
 
 // RUN: not %clang -target armv8m.main-eabi -mpure-code -mno-movt %s 2>&1 \
 // RUN:    | FileCheck %s -check-prefix CHECK-PURE-CODE-NO-MOVT
 // CHECK-PURE-CODE-NO-MOVT: error: option '-mpure-code' cannot be specified with '-mno-movt'
-
-// RUN: not %clang -target armv8m.main-eabi -mpure-code -mlong-calls %s 2>&1 \
-// RUN:    | FileCheck %s -check-prefix CHECK-PURE-CODE-LONG-CALLS
-// CHECK-PURE-CODE-LONG-CALLS: error: option '-mpure-code' cannot be specified with '-mlong-calls'

llvm/lib/Target/ARM/ARMISelLowering.cpp

ARMTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
 
   // If the callee is a GlobalAddress/ExternalSymbol node (quite common, every
   // direct call is) turn it into a TargetGlobalAddress/TargetExternalSymbol
   // node so that legalize doesn't hack it.
   bool isDirect = false;
 
   const TargetMachine &TM = getTargetMachine();
   const Module *Mod = MF.getFunction().getParent();
-  const GlobalValue *GV = nullptr;
+  const GlobalValue *GVal = nullptr;
   if (GlobalAddressSDNode *G = dyn_cast<GlobalAddressSDNode>(Callee))
-    GV = G->getGlobal();
+    GVal = G->getGlobal();
   bool isStub =
-      !TM.shouldAssumeDSOLocal(*Mod, GV) && Subtarget->isTargetMachO();
+      !TM.shouldAssumeDSOLocal(*Mod, GVal) && Subtarget->isTargetMachO();
 
   bool isARMFunc = !Subtarget->isThumb() || (isStub && !Subtarget->isMClass());
   bool isLocalARMFunc = false;
   auto PtrVt = getPointerTy(DAG.getDataLayout());
 
   if (Subtarget->genLongCalls()) {
     assert((!isPositionIndependent() || Subtarget->isTargetWindows()) &&
            "long-calls codegen is not position independent!");
     // Handle a global address or an external symbol. If it's not one of
     // those, the target's already in a register, so we don't need to do
     // anything extra.
     if (isa<GlobalAddressSDNode>(Callee)) {
+      SDValue Addr;
+
+      // When generating execute-only code we promote constant pools to
+      // the global data section, the same as in `LowerConstantPool`.
+      if (Subtarget->genExecuteOnly()) {
+        Module *M = const_cast<Module *>(
+            DAG.getMachineFunction().getFunction().getParent());
+        Constant *C =
+            const_cast<Constant *>(static_cast<const Constant *>(GVal));
+        GlobalVariable *GVar = new GlobalVariable(
+            *M, GVal->getType(), false, GlobalVariable::InternalLinkage, C,
+            Twine(DAG.getDataLayout().getPrivateGlobalPrefix()) + "CP" +
+                Twine(DAG.getMachineFunction().getFunctionNumber()) + "_" +
+                Twine(AFI->createPICLabelUId()));
+        SDValue GA =
+            DAG.getTargetGlobalAddress(dyn_cast<GlobalValue>(GVar), dl, PtrVt);
+        Addr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32,
+                           LowerGlobalAddressELF(GA, DAG));
+      } else {
         // Create a constant pool entry for the callee address
         unsigned ARMPCLabelIndex = AFI->createPICLabelUId();
-      ARMConstantPoolValue *CPV =
-        ARMConstantPoolConstant::Create(GV, ARMPCLabelIndex, ARMCP::CPValue, 0);
+        ARMConstantPoolValue *CPV = ARMConstantPoolConstant::Create(
+            GVal, ARMPCLabelIndex, ARMCP::CPValue, 0);
 
         // Get the address of the callee into a register
-      SDValue CPAddr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4));
-      CPAddr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, CPAddr);
+        Addr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4));
+        Addr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, Addr);
+      }
+
       Callee = DAG.getLoad(
-          PtrVt, dl, DAG.getEntryNode(), CPAddr,
+          PtrVt, dl, DAG.getEntryNode(), Addr,
           MachinePointerInfo::getConstantPool(DAG.getMachineFunction()));
     } else if (ExternalSymbolSDNode *S=dyn_cast<ExternalSymbolSDNode>(Callee)) {
       const char *Sym = S->getSymbol();
+      SDValue Addr;
 
+      // When generating execute-only code we promote constant pools to
+      // the global data section, the same as in `LowerConstantPool`.
+      if (Subtarget->genExecuteOnly()) {
+        Module *M = const_cast<Module *>(
+            DAG.getMachineFunction().getFunction().getParent());
+        Constant *C =
+            const_cast<Constant *>(static_cast<const Constant *>(GVal));
+        GlobalVariable *GVar = new GlobalVariable(
+            *M, GVal->getType(), false, GlobalVariable::InternalLinkage, C,
+            Twine(DAG.getDataLayout().getPrivateGlobalPrefix()) + "CP" +
+                Twine(DAG.getMachineFunction().getFunctionNumber()) + "_" +
+                Twine(AFI->createPICLabelUId()));
+        SDValue GA =
+            DAG.getTargetGlobalAddress(dyn_cast<GlobalValue>(GVar), dl, PtrVt);
+        Addr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32,
+                           LowerGlobalAddressELF(GA, DAG));
+      } else {
         // Create a constant pool entry for the callee address
         unsigned ARMPCLabelIndex = AFI->createPICLabelUId();
-      ARMConstantPoolValue *CPV =
-        ARMConstantPoolSymbol::Create(*DAG.getContext(), Sym,
-                                      ARMPCLabelIndex, 0);
+        ARMConstantPoolValue *CPV = ARMConstantPoolSymbol::Create(
+            *DAG.getContext(), Sym, ARMPCLabelIndex, 0);
+
         // Get the address of the callee into a register
-      SDValue CPAddr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4));
-      CPAddr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, CPAddr);
+        Addr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4));
+        Addr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, Addr);
+      }
+
       Callee = DAG.getLoad(
-          PtrVt, dl, DAG.getEntryNode(), CPAddr,
+          PtrVt, dl, DAG.getEntryNode(), Addr,
           MachinePointerInfo::getConstantPool(DAG.getMachineFunction()));
     }
   } else if (isa<GlobalAddressSDNode>(Callee)) {
     if (!PreferIndirect) {
       isDirect = true;
-      bool isDef = GV->isStrongDefinitionForLinker();
+      bool isDef = GVal->isStrongDefinitionForLinker();
 
       // ARM call to a local ARM function is predicable.
       isLocalARMFunc = !Subtarget->isThumb() && (isDef || !ARMInterworking);
       // tBX takes a register source operand.
       if (isStub && Subtarget->isThumb1Only() && !Subtarget->hasV5TOps()) {
         assert(Subtarget->isTargetMachO() && "WrapperPIC use on non-MachO?");
         Callee = DAG.getNode(
             ARMISD::WrapperPIC, dl, PtrVt,
-            DAG.getTargetGlobalAddress(GV, dl, PtrVt, 0, ARMII::MO_NONLAZY));
+            DAG.getTargetGlobalAddress(GVal, dl, PtrVt, 0, ARMII::MO_NONLAZY));
         Callee = DAG.getLoad(
             PtrVt, dl, DAG.getEntryNode(), Callee,
             MachinePointerInfo::getGOT(DAG.getMachineFunction()), MaybeAlign(),
             MachineMemOperand::MODereferenceable |
                 MachineMemOperand::MOInvariant);
       } else if (Subtarget->isTargetCOFF()) {
         assert(Subtarget->isTargetWindows() &&
                "Windows is the only supported COFF target");
         unsigned TargetFlags = ARMII::MO_NO_FLAG;
-        if (GV->hasDLLImportStorageClass())
+        if (GVal->hasDLLImportStorageClass())
           TargetFlags = ARMII::MO_DLLIMPORT;
-        else if (!TM.shouldAssumeDSOLocal(*GV->getParent(), GV))
+        else if (!TM.shouldAssumeDSOLocal(*GVal->getParent(), GVal))
           TargetFlags = ARMII::MO_COFFSTUB;
-        Callee = DAG.getTargetGlobalAddress(GV, dl, PtrVt, /*offset=*/0,
+        Callee = DAG.getTargetGlobalAddress(GVal, dl, PtrVt, /*offset=*/0,
                                             TargetFlags);
         if (TargetFlags & (ARMII::MO_DLLIMPORT | ARMII::MO_COFFSTUB))
           Callee =
               DAG.getLoad(PtrVt, dl, DAG.getEntryNode(),
                           DAG.getNode(ARMISD::Wrapper, dl, PtrVt, Callee),
                           MachinePointerInfo::getGOT(DAG.getMachineFunction()));
       } else {
-        Callee = DAG.getTargetGlobalAddress(GV, dl, PtrVt, 0, 0);
+        Callee = DAG.getTargetGlobalAddress(GVal, dl, PtrVt, 0, 0);
       }
     }
   } else if (ExternalSymbolSDNode *S = dyn_cast<ExternalSymbolSDNode>(Callee)) {
     isDirect = true;
     // tBX takes a register source operand.
     const char *Sym = S->getSymbol();
     if (isARMFunc && Subtarget->isThumb1Only() && !Subtarget->hasV5TOps()) {
       unsigned ARMPCLabelIndex = AFI->createPICLabelUId();

llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll

+; RUN: llc < %s -mtriple=thumbv7em-arm-none-eabi -relocation-model=static | FileCheck %s -check-prefixes=CHECK,STATIC
+; RUN: llc < %s -mtriple=thumbv7em-arm-none-eabi -relocation-model=rwpi | FileCheck %s -check-prefixes=CHECK,RWPI
+
+define void @fn() #0 {
+entry:
+; CHECK-LABEL: fn:
+; CHECK:       ldr [[REG:r[0-9]+]], .LCPI0_0
+; CHECK-NEXT:  blx [[REG]]
+; CHECK:       .LCPI0_0:
+; CHECK-NEXT:  .long   bar
+  call void @bar()
+  ret void
+}
+
+define void @execute_only_fn() #1 {
+; STATIC-LABEL: execute_only_fn:
+; STATIC:       movw    [[REG0:r[0-9]+]], :lower16:.LCP1_0
+; STATIC-NEXT:  movt    [[REG0]], :upper16:.LCP1_0
+; STATIC-NEXT:  ldr     [[REG1:r[0-9]+]], [[[REG0]]]
+; STATIC-NEXT:  blx     [[REG1]]
+; STATIC-NOT:   .LCPI0_0:
+
+; RWPI-LABEL: execute_only_fn:
+; RWPI:       movw    [[REG0:r[0-9]+]], :lower16:.LCP1_0(sbrel)
+; RWPI-NEXT:  movt    [[REG0]], :upper16:.LCP1_0(sbrel)
+; RWPI-NEXT:  add     [[REG0]], r9
+; RWPI-NEXT:  ldr     [[REG1:r[0-9]+]], [[[REG0]]]
+; RWPI-NEXT:  blx     [[REG1]]
+; RWPI-NOT:   .LCPI0_0:
+entry:
+  call void @bar()
+  ret void
+}
+
+attributes #0 = { noinline optnone "target-features"="+thumb-mode,+long-calls" }
+attributes #1 = { noinline optnone "target-features"="+execute-only,+thumb-mode,+long-calls" }
+
+declare dso_local void @bar()