[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.
ZhiyaoMa98 created this revision. ZhiyaoMa98 added reviewers: labrinea, rengolin. Herald added subscribers: hiraditya, kristof.beyls. Herald added a project: All. ZhiyaoMa98 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added projects: clang, LLVM. Promote constant pools used by long-calls to global variables. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D136203 Files: clang/lib/Driver/ToolChains/Arch/ARM.cpp clang/test/Driver/arm-execute-only.c llvm/lib/Target/ARM/ARMISelLowering.cpp llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll Index: llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll === --- /dev/null +++ llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll @@ -0,0 +1,38 @@ +; 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() Index: llvm/lib/Target/ARM/ARMISelLowering.cpp === --- llvm/lib/Target/ARM/ARMISelLowering.cpp +++ llvm/lib/Target/ARM/ARMISelLowering.cpp @@ -2630,11 +2630,11 @@ const TargetMachine &TM = getTargetMachine(); const Module *Mod = MF.getFunction().getParent(); - const GlobalValue *GV = nullptr; + const GlobalValue *GVal = nullptr; if (GlobalAddressSDNode *G = dyn_cast(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; @@ -2647,36 +2647,77 @@ // those, the target's already in a register, so we don't need to do // anything extra. if (isa(Callee)) { - // Create a constant pool entry for the callee address - unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); - ARMConstantPoolValue *CPV = -ARMConstantPoolConstant::Create(GV, ARMPCLabelIndex, ARMCP::CPValue, 0); + 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( +DAG.getMachineFunction().getFunction().getParent()); +Constant *C = +const_cast(static_cast(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(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( +GVal, ARMPCLabelIndex, ARMCP::CPValue, 0); + +// Get the address of the callee into a register +Addr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4)); +Addr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, Addr); + } - // 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); Callee = DAG.getLoad( - PtrVt, dl, DAG.getEntryNode(), CPAddr, +
[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.
ZhiyaoMa98 added a comment. @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? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136203/new/ https://reviews.llvm.org/D136203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.
ZhiyaoMa98 added a comment. > 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) // Absolute address is held in RAM now. 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] 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? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136203/new/ https://reviews.llvm.org/D136203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.
ZhiyaoMa98 added a comment. 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`: : 0: 4800ldr r0, [pc, #0]; (4 ) 2: 4700bx r0 4: .word 0x ARM GNU with `arm-none-eabi-gcc -O2 -fPIE -mlong-calls -msingle-pic-base -mcpu=cortex-m4 -c test.c`: : 0: 4b01ldr r3, [pc, #4]; (8 ) 2: f859 3003 ldr.w r3, [r9, r3] 6: 4718bx r3 8: .word 0x Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136203/new/ https://reviews.llvm.org/D136203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.
ZhiyaoMa98 updated this revision to Diff 468928. ZhiyaoMa98 edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136203/new/ https://reviews.llvm.org/D136203 Files: clang/lib/Driver/ToolChains/Arch/ARM.cpp clang/test/Driver/arm-execute-only.c llvm/lib/Target/ARM/ARMISelLowering.cpp llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll Index: llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll === --- /dev/null +++ llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll @@ -0,0 +1,35 @@ +; 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:bar +; STATIC-NEXT: movt[[REG0]], :upper16:bar +; STATIC-NEXT: blx [[REG0]] +; STATIC-NOT: .LCPI0_0: + +; RWPI-LABEL: execute_only_fn: +; RWPI: movw[[REG0:r[0-9]+]], :lower16:bar +; RWPI-NEXT: movt[[REG0]], :upper16:bar +; RWPI-NEXT: blx [[REG0]] +; 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() Index: llvm/lib/Target/ARM/ARMISelLowering.cpp === --- llvm/lib/Target/ARM/ARMISelLowering.cpp +++ llvm/lib/Target/ARM/ARMISelLowering.cpp @@ -2630,11 +2630,11 @@ const TargetMachine &TM = getTargetMachine(); const Module *Mod = MF.getFunction().getParent(); - const GlobalValue *GV = nullptr; + const GlobalValue *GVal = nullptr; if (GlobalAddressSDNode *G = dyn_cast(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; @@ -2647,36 +2647,59 @@ // those, the target's already in a register, so we don't need to do // anything extra. if (isa(Callee)) { - // Create a constant pool entry for the callee address - unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); - ARMConstantPoolValue *CPV = -ARMConstantPoolConstant::Create(GV, 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); - Callee = DAG.getLoad( - PtrVt, dl, DAG.getEntryNode(), CPAddr, - MachinePointerInfo::getConstantPool(DAG.getMachineFunction())); + SDValue Addr; + + // When generating execute-only code we use movw movt pair. + // Currently execute-only is only available for architectures that + // support movw movt, so we are safe to assume that. + if (Subtarget->genExecuteOnly()) { +SDValue GA = DAG.getTargetGlobalAddress(GVal, dl, PtrVt); +++NumMovwMovt; +Callee = DAG.getNode(ARMISD::Wrapper, dl, PtrVt, + DAG.getTargetGlobalAddress(GVal, dl, PtrVt)); + } else { +// Create a constant pool entry for the callee address +unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); +ARMConstantPoolValue *CPV = ARMConstantPoolConstant::Create( +GVal, ARMPCLabelIndex, ARMCP::CPValue, 0); + +// Get the address of the callee into a register +Addr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4)); +Addr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, Addr); +Callee = DAG.getLoad( +PtrVt, dl, DAG.getEntryNode(), Addr, +MachinePointerInfo::getConstantPool(DAG.getMachineFunction())); + } } else if (ExternalSymbolSDNode *S=dyn_cast(Callee)) { const char *Sym = S->getSymbol(); - - // Create a constant pool entry for the callee address - unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); - 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); - Callee = DAG
[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.
ZhiyaoMa98 marked an inline comment as done. ZhiyaoMa98 added a comment. 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? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136203/new/ https://reviews.llvm.org/D136203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.
ZhiyaoMa98 updated this revision to Diff 468957. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136203/new/ https://reviews.llvm.org/D136203 Files: clang/lib/Driver/ToolChains/Arch/ARM.cpp clang/test/Driver/arm-execute-only.c llvm/lib/Target/ARM/ARMISelLowering.cpp llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll Index: llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll === --- /dev/null +++ llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll @@ -0,0 +1,35 @@ +; 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:bar +; STATIC-NEXT: movt[[REG0]], :upper16:bar +; STATIC-NEXT: blx [[REG0]] +; STATIC-NOT: .LCPI0_0: + +; RWPI-LABEL: execute_only_fn: +; RWPI: movw[[REG0:r[0-9]+]], :lower16:bar +; RWPI-NEXT: movt[[REG0]], :upper16:bar +; RWPI-NEXT: blx [[REG0]] +; 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() Index: llvm/lib/Target/ARM/ARMISelLowering.cpp === --- llvm/lib/Target/ARM/ARMISelLowering.cpp +++ llvm/lib/Target/ARM/ARMISelLowering.cpp @@ -2630,11 +2630,11 @@ const TargetMachine &TM = getTargetMachine(); const Module *Mod = MF.getFunction().getParent(); - const GlobalValue *GV = nullptr; + const GlobalValue *GVal = nullptr; if (GlobalAddressSDNode *G = dyn_cast(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; @@ -2647,36 +2647,63 @@ // those, the target's already in a register, so we don't need to do // anything extra. if (isa(Callee)) { - // Create a constant pool entry for the callee address - unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); - ARMConstantPoolValue *CPV = -ARMConstantPoolConstant::Create(GV, 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); - Callee = DAG.getLoad( - PtrVt, dl, DAG.getEntryNode(), CPAddr, - MachinePointerInfo::getConstantPool(DAG.getMachineFunction())); + SDValue Addr; + + // When generating execute-only code we use movw movt pair. + // Currently execute-only is only available for architectures that + // support movw movt, so we are safe to assume that. + if (Subtarget->genExecuteOnly()) { +assert(Subtarget->useMovt() && + "long-calls with execute-only requires movt and movw!"); +SDValue GA = DAG.getTargetGlobalAddress(GVal, dl, PtrVt); +++NumMovwMovt; +Callee = DAG.getNode(ARMISD::Wrapper, dl, PtrVt, + DAG.getTargetGlobalAddress(GVal, dl, PtrVt)); + } else { +// Create a constant pool entry for the callee address +unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); +ARMConstantPoolValue *CPV = ARMConstantPoolConstant::Create( +GVal, ARMPCLabelIndex, ARMCP::CPValue, 0); + +// Get the address of the callee into a register +Addr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4)); +Addr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, Addr); +Callee = DAG.getLoad( +PtrVt, dl, DAG.getEntryNode(), Addr, +MachinePointerInfo::getConstantPool(DAG.getMachineFunction())); + } } else if (ExternalSymbolSDNode *S=dyn_cast(Callee)) { const char *Sym = S->getSymbol(); - - // Create a constant pool entry for the callee address - unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); - 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.ge
[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.
ZhiyaoMa98 added a comment. > 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136203/new/ https://reviews.llvm.org/D136203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.
ZhiyaoMa98 updated this revision to Diff 468973. ZhiyaoMa98 marked an inline comment as done. ZhiyaoMa98 added a comment. Updated the comment to reflect that now we allow using `-mlong-calls` with `-mexecute-only`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136203/new/ https://reviews.llvm.org/D136203 Files: clang/lib/Driver/ToolChains/Arch/ARM.cpp clang/test/Driver/arm-execute-only.c llvm/lib/Target/ARM/ARMISelLowering.cpp llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll Index: llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll === --- /dev/null +++ llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll @@ -0,0 +1,35 @@ +; 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:bar +; STATIC-NEXT: movt[[REG0]], :upper16:bar +; STATIC-NEXT: blx [[REG0]] +; STATIC-NOT: .LCPI0_0: + +; RWPI-LABEL: execute_only_fn: +; RWPI: movw[[REG0:r[0-9]+]], :lower16:bar +; RWPI-NEXT: movt[[REG0]], :upper16:bar +; RWPI-NEXT: blx [[REG0]] +; 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() Index: llvm/lib/Target/ARM/ARMISelLowering.cpp === --- llvm/lib/Target/ARM/ARMISelLowering.cpp +++ llvm/lib/Target/ARM/ARMISelLowering.cpp @@ -2630,11 +2630,11 @@ const TargetMachine &TM = getTargetMachine(); const Module *Mod = MF.getFunction().getParent(); - const GlobalValue *GV = nullptr; + const GlobalValue *GVal = nullptr; if (GlobalAddressSDNode *G = dyn_cast(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; @@ -2647,36 +2647,63 @@ // those, the target's already in a register, so we don't need to do // anything extra. if (isa(Callee)) { - // Create a constant pool entry for the callee address - unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); - ARMConstantPoolValue *CPV = -ARMConstantPoolConstant::Create(GV, 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); - Callee = DAG.getLoad( - PtrVt, dl, DAG.getEntryNode(), CPAddr, - MachinePointerInfo::getConstantPool(DAG.getMachineFunction())); + SDValue Addr; + + // When generating execute-only code we use movw movt pair. + // Currently execute-only is only available for architectures that + // support movw movt, so we are safe to assume that. + if (Subtarget->genExecuteOnly()) { +assert(Subtarget->useMovt() && + "long-calls with execute-only requires movt and movw!"); +SDValue GA = DAG.getTargetGlobalAddress(GVal, dl, PtrVt); +++NumMovwMovt; +Callee = DAG.getNode(ARMISD::Wrapper, dl, PtrVt, + DAG.getTargetGlobalAddress(GVal, dl, PtrVt)); + } else { +// Create a constant pool entry for the callee address +unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); +ARMConstantPoolValue *CPV = ARMConstantPoolConstant::Create( +GVal, ARMPCLabelIndex, ARMCP::CPValue, 0); + +// Get the address of the callee into a register +Addr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4)); +Addr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, Addr); +Callee = DAG.getLoad( +PtrVt, dl, DAG.getEntryNode(), Addr, +MachinePointerInfo::getConstantPool(DAG.getMachineFunction())); + } } else if (ExternalSymbolSDNode *S=dyn_cast(Callee)) { const char *Sym = S->getSymbol(); - - // Create a constant pool entry for the callee address - unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); - ARMConstantPoolValue *CPV = -ARMConstantPoolSymbol::Create(*DAG.getContext(), Sym, - ARM
[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.
ZhiyaoMa98 updated this revision to Diff 469049. ZhiyaoMa98 marked an inline comment as done. ZhiyaoMa98 added a comment. Remove the unused `GA` variable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136203/new/ https://reviews.llvm.org/D136203 Files: clang/lib/Driver/ToolChains/Arch/ARM.cpp clang/test/Driver/arm-execute-only.c llvm/lib/Target/ARM/ARMISelLowering.cpp llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll Index: llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll === --- /dev/null +++ llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll @@ -0,0 +1,35 @@ +; 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:bar +; STATIC-NEXT: movt[[REG0]], :upper16:bar +; STATIC-NEXT: blx [[REG0]] +; STATIC-NOT: .LCPI0_0: + +; RWPI-LABEL: execute_only_fn: +; RWPI: movw[[REG0:r[0-9]+]], :lower16:bar +; RWPI-NEXT: movt[[REG0]], :upper16:bar +; RWPI-NEXT: blx [[REG0]] +; 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() Index: llvm/lib/Target/ARM/ARMISelLowering.cpp === --- llvm/lib/Target/ARM/ARMISelLowering.cpp +++ llvm/lib/Target/ARM/ARMISelLowering.cpp @@ -2630,11 +2630,11 @@ const TargetMachine &TM = getTargetMachine(); const Module *Mod = MF.getFunction().getParent(); - const GlobalValue *GV = nullptr; + const GlobalValue *GVal = nullptr; if (GlobalAddressSDNode *G = dyn_cast(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; @@ -2647,36 +2647,61 @@ // those, the target's already in a register, so we don't need to do // anything extra. if (isa(Callee)) { - // Create a constant pool entry for the callee address - unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); - ARMConstantPoolValue *CPV = -ARMConstantPoolConstant::Create(GV, 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); - Callee = DAG.getLoad( - PtrVt, dl, DAG.getEntryNode(), CPAddr, - MachinePointerInfo::getConstantPool(DAG.getMachineFunction())); + SDValue Addr; + + // When generating execute-only code we use movw movt pair. + // Currently execute-only is only available for architectures that + // support movw movt, so we are safe to assume that. + if (Subtarget->genExecuteOnly()) { +assert(Subtarget->useMovt() && + "long-calls with execute-only requires movt and movw!"); +++NumMovwMovt; +Callee = DAG.getNode(ARMISD::Wrapper, dl, PtrVt, + DAG.getTargetGlobalAddress(GVal, dl, PtrVt)); + } else { +// Create a constant pool entry for the callee address +unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); +ARMConstantPoolValue *CPV = ARMConstantPoolConstant::Create( +GVal, ARMPCLabelIndex, ARMCP::CPValue, 0); + +// Get the address of the callee into a register +Addr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4)); +Addr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, Addr); +Callee = DAG.getLoad( +PtrVt, dl, DAG.getEntryNode(), Addr, +MachinePointerInfo::getConstantPool(DAG.getMachineFunction())); + } } else if (ExternalSymbolSDNode *S=dyn_cast(Callee)) { const char *Sym = S->getSymbol(); - - // Create a constant pool entry for the callee address - unsigned ARMPCLabelIndex = AFI->createPICLabelUId(); - ARMConstantPoolValue *CPV = -ARMConstantPoolSymbol::Create(*DAG.getContext(), Sym, - ARMPCLabelIndex, 0); - // Get the address of the callee into a register - SDValue CPAddr = DAG.getTargetConstantPool(CPV,
[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.
ZhiyaoMa98 added a comment. Just in case you assume that I have push permission, unfortunately I do not. Could you help me merge the patch in? Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136203/new/ https://reviews.llvm.org/D136203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D126346: [Clang] Make PIC check comprehensive with respect to relocation models.
ZhiyaoMa98 created this revision. ZhiyaoMa98 added reviewers: rsmith, llvm-commits. Herald added a project: All. ZhiyaoMa98 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add missing if branch for "ropi", "rwpi" and "ropi-rwpi" relocation model when initializing the PIC boolean variable. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D126346 Files: clang/tools/driver/cc1as_main.cpp Index: clang/tools/driver/cc1as_main.cpp === --- clang/tools/driver/cc1as_main.cpp +++ clang/tools/driver/cc1as_main.cpp @@ -403,6 +403,12 @@ PIC = false; } else if (Opts.RelocationModel == "pic") { PIC = true; + } else if (Opts.RelocationModel == "ropi-rwpi") { +PIC = true; + } else if (Opts.RelocationModel == "ropi") { +PIC = false; + } else if (Opts.RelocationModel == "rwpi") { +PIC = false; } else { assert(Opts.RelocationModel == "dynamic-no-pic" && "Invalid PIC model!"); Index: clang/tools/driver/cc1as_main.cpp === --- clang/tools/driver/cc1as_main.cpp +++ clang/tools/driver/cc1as_main.cpp @@ -403,6 +403,12 @@ PIC = false; } else if (Opts.RelocationModel == "pic") { PIC = true; + } else if (Opts.RelocationModel == "ropi-rwpi") { +PIC = true; + } else if (Opts.RelocationModel == "ropi") { +PIC = false; + } else if (Opts.RelocationModel == "rwpi") { +PIC = false; } else { assert(Opts.RelocationModel == "dynamic-no-pic" && "Invalid PIC model!"); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits