[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.

2022-10-18 Thread Zhiyao Ma via Phabricator via cfe-commits
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.

2022-10-18 Thread Zhiyao Ma via Phabricator via cfe-commits
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.

2022-10-18 Thread Zhiyao Ma via Phabricator via cfe-commits
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.

2022-10-18 Thread Zhiyao Ma via Phabricator via cfe-commits
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.

2022-10-19 Thread Zhiyao Ma via Phabricator via cfe-commits
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.

2022-10-19 Thread Zhiyao Ma via Phabricator via cfe-commits
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.

2022-10-19 Thread Zhiyao Ma via Phabricator via cfe-commits
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.

2022-10-19 Thread Zhiyao Ma via Phabricator via cfe-commits
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.

2022-10-19 Thread Zhiyao Ma via Phabricator via cfe-commits
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.

2022-10-19 Thread Zhiyao Ma via Phabricator via cfe-commits
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.

2022-10-20 Thread Zhiyao Ma via Phabricator via cfe-commits
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.

2022-05-24 Thread Zhiyao Ma via Phabricator via cfe-commits
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