[PATCH] D114116: [clang][ARM] relax -mtp=cp15 for ARMv6 non-thumb cases

2021-11-18 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:155
+  llvm::ARM::ArchKind AK = llvm::ARM::parseArch(Triple.getArchName());
+  return Ver >= 7 || AK == llvm::ARM::ArchKind::ARMV6T2 ||
+ (Ver == 6 && Triple.isARM());

peter.smith wrote:
> Are we restricting based on whether the threadid register is present or 
> whether the instructions are available to access the cp15 registers?
> 
> If we're going by whether the CPU has the register present then it will be
> * A and R profile (not M profile, even the ones that have Thumb2)
> * V6K (includes ARM1176 but not ARM1156t2-s which has Thumb-2!) and V7+ (A 
> and R profile)
> 
> If we're going by the instructions to write to CP15 then it is:
> * Arm state (everything)
> * Thumb2 (v7 + v6t2)
> 
> The above seems to be a blend of the two. Is it worth choosing one form or 
> the other? GCC seems to use the latter. I guess using this option falls into 
> the I know what I'm doing area that accessing named system registers comes 
> into. If the kernel supports it the stricter version may help catch more 
> mistakes though.
> 
> The v7 A/R Arm ARM https://developer.arm.com/documentation/ddi0403/ed
> has in `D12.7.21 CP15 c13, Context ID support`
> ``` An ARMv6K implementation requires the Software Thread ID registers 
> described in VMSA CP15 c13
> register summary, Process, context and thread ID registers on page B3-1474. 
> ```
> 
> The Arm 1156-s (the only v6t2 processor) TRM 
> https://developer.arm.com/documentation/ddi0338/g/system-control-coprocessor/system-control-processor-registers/c13--process-id-register?lang=en
>  which shows only one process ID register under opcode 1 accessed via:
> ```
> MRC p15, 0, , c13, c0, 1 ;Read Process ID Register
> ```
> Whereas the ThreadID register is opcode 3 on CPUs that are v6k and v7.
The primary reason for tightening these checks was to avoid an assert in the 
backend when using -mtp=cp15 with a Thumb1 target, so I'd say this is more 
about whether the ISA has the opcode to begin with, rather than whether CPU x 
implements it or not.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114116/new/

https://reviews.llvm.org/D114116

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113026: [ARM] reject -mtp=cp15 if target subarch does not support it

2021-11-09 Thread Ard Biesheuvel via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
ardb marked an inline comment as done.
Closed by commit rG24772720c545: [ARM] reject -mtp=cp15 if target subarch does 
not support it (authored by ardb).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113026/new/

https://reviews.llvm.org/D113026

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/clang-translation.c


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -110,15 +110,27 @@
 // ARMV5E: "-cc1"
 // ARMV5E: "-target-cpu" "arm1022e"
 
-// RUN: %clang -target arm-linux -mtp=cp15 -### -S %s -arch armv7 2>&1 | \
+// RUN: %clang -target armv7-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER-HARD %s
 // ARMv7_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
 
-// RUN: %clang -target arm-linux -mtp=soft -### -S %s -arch armv7 2>&1 | \
+// RUN: %clang -target armv6t2-linux -mtp=cp15 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv6T2_THREAD_POINTER-HARD %s
+// ARMv6T2_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
+
+// RUN: %clang -target armv5t-linux -mtp=cp15 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv5_THREAD_POINTER_UNSUPP %s
+// ARMv5_THREAD_POINTER_UNSUPP: hardware TLS register is not supported for the 
armv5 sub-architecture
+
+// RUN: %clang -target thumbv6-linux -mtp=cp15 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv6_THREAD_POINTER_UNSUPP %s
+// ARMv6_THREAD_POINTER_UNSUPP: hardware TLS register is not supported for the 
armv6 sub-architecture
+
+// RUN: %clang -target armv7-linux -mtp=soft -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_SOFT %s
 // ARMv7_THREAD_POINTER_SOFT-NOT: "-target-feature" "+read-tp-hard"
 
-// RUN: %clang -target arm-linux -### -S %s -arch armv7 2>&1 | \
+// RUN: %clang -target armv7-linux -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_NON %s
 // ARMv7_THREAD_POINTER_NON-NOT: "-target-feature" "+read-tp-hard"
 
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -53,7 +53,8 @@
 const llvm::opt::ArgList );
 void setFloatABIInTriple(const Driver , const llvm::opt::ArgList ,
  llvm::Triple );
-ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList );
+ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList ,
+ const llvm::Triple );
 void setArchNameInTriple(const Driver , const llvm::opt::ArgList ,
  types::ID InputType, llvm::Triple );
 
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -148,13 +148,21 @@
 }
 
 // Select mode for reading thread pointer (-mtp=soft/cp15).
-arm::ReadTPMode arm::getReadTPMode(const Driver , const ArgList ) {
+arm::ReadTPMode arm::getReadTPMode(const Driver , const ArgList ,
+   const llvm::Triple ) {
   if (Arg *A = Args.getLastArg(options::OPT_mtp_mode_EQ)) {
 arm::ReadTPMode ThreadPointer =
 llvm::StringSwitch(A->getValue())
 .Case("cp15", ReadTPMode::Cp15)
 .Case("soft", ReadTPMode::Soft)
 .Default(ReadTPMode::Invalid);
+if (ThreadPointer == ReadTPMode::Cp15 &&
+getARMSubArchVersionNumber(Triple) < 7 &&
+llvm::ARM::parseArch(Triple.getArchName()) !=
+llvm::ARM::ArchKind::ARMV6T2) {
+  D.Diag(diag::err_target_unsupported_tp_hard) << Triple.getArchName();
+  return ReadTPMode::Invalid;
+}
 if (ThreadPointer != ReadTPMode::Invalid)
   return ThreadPointer;
 if (StringRef(A->getValue()).empty())
@@ -422,7 +430,7 @@
   bool KernelOrKext =
   Args.hasArg(options::OPT_mkernel, options::OPT_fapple_kext);
   arm::FloatABI ABI = arm::getARMFloatABI(D, Triple, Args);
-  arm::ReadTPMode ThreadPointer = arm::getReadTPMode(D, Args);
+  arm::ReadTPMode ThreadPointer = arm::getReadTPMode(D, Args, Triple);
   llvm::Optional> WaCPU, WaFPU, WaHDiv,
   WaArch;
 


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -110,15 +110,27 @@
 // ARMV5E: "-cc1"
 // ARMV5E: "-target-cpu" "arm1022e"
 
-// RUN: %clang -target arm-linux -mtp=cp15 -### -S %s -arch armv7 2>&1 | \
+// RUN: %clang -target armv7-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER-HARD

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

2021-11-09 Thread Ard Biesheuvel via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
ardb marked an inline comment as done.
Closed by commit rGa19da876ab93: [ARM] implement support for TLS register based 
stack protector (authored by ardb).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112768/new/

https://reviews.llvm.org/D112768

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
  llvm/lib/Target/ARM/ARMInstrInfo.cpp
  llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
  llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
  llvm/test/CodeGen/ARM/stack-guard-tls.ll

Index: llvm/test/CodeGen/ARM/stack-guard-tls.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/stack-guard-tls.ll
@@ -0,0 +1,38 @@
+; RUN: split-file %s %t
+; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
+; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll
+; RUN: llc %t/a2.ll -mtriple=armv7-unknown-linux-gnueabihf -mattr=+read-tp-hard -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-SMALL %s
+; RUN: llc %t/a2.ll -mtriple=thumbv7-unknown-linux-gnueabihf -mattr=+read-tp-hard -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-SMALL %s
+; RUN: llc %t/b2.ll -mtriple=armv7-unknown-linux-gnueabihf -mattr=+read-tp-hard -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-LARGE %s
+; RUN: llc %t/b2.ll -mtriple=thumbv7-unknown-linux-gnueabihf -mattr=+read-tp-hard -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-LARGE %s
+
+;--- main.ll
+declare void @baz(i32*)
+
+define void @foo(i64 %t) sspstrong {
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+!llvm.module.flags = !{!1, !2}
+!1 = !{i32 2, !"stack-protector-guard", !"tls"}
+
+;--- a.ll
+!2 = !{i32 2, !"stack-protector-guard-offset", i32 1296}
+
+;--- b.ll
+!2 = !{i32 2, !"stack-protector-guard-offset", i32 4296}
+
+; CHECK: mrc p15, #0, [[REG1:r[0-9]+]], c13, c0, #3
+; CHECK-SMALL-NEXT: ldr{{(\.w)?}} [[REG1]], {{\[}}[[REG1]], #1296]
+; CHECK-LARGE-NEXT: add{{(\.w)?}} [[REG1]], [[REG1]], #4096
+; CHECK-LARGE-NEXT: ldr{{(\.w)?}} [[REG1]], {{\[}}[[REG1]], #200]
+; CHECK: bl baz
+; CHECK: mrc p15, #0, [[REG2:r[0-9]+]], c13, c0, #3
+; CHECK-SMALL-NEXT: ldr{{(\.w)?}} [[REG2]], {{\[}}[[REG2]], #1296]
+; CHECK-LARGE-NEXT: add{{(\.w)?}} [[REG2]], [[REG2]], #4096
+; CHECK-LARGE-NEXT: ldr{{(\.w)?}} [[REG2]], {{\[}}[[REG2]], #200]
Index: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
===
--- llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
+++ llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
@@ -250,6 +250,13 @@
 void Thumb2InstrInfo::expandLoadStackGuard(
 MachineBasicBlock::iterator MI) const {
   MachineFunction  = *MI->getParent()->getParent();
+  Module  = *MF.getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::t2MRC, ARM::t2LDRi12);
+return;
+  }
+
   const GlobalValue *GV =
   cast((*MI->memoperands_begin())->getValue());
 
Index: llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
===
--- llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
+++ llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
@@ -135,6 +135,11 @@
 MachineBasicBlock::iterator MI) const {
   MachineFunction  = *MI->getParent()->getParent();
   const TargetMachine  = MF.getTarget();
+  Module  = *MF.getFunction().getParent();
+
+  assert(M.getStackProtectorGuard() != "tls" &&
+ "TLS stack protector not supported for Thumb1 targets");
+
   if (TM.isPositionIndependent())
 expandLoadStackGuardBase(MI, ARM::tLDRLIT_ga_pcrel, ARM::tLDRi);
   else
Index: llvm/lib/Target/ARM/ARMInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMInstrInfo.cpp
@@ -95,6 +95,12 @@
   MachineFunction  = *MI->getParent()->getParent();
   const ARMSubtarget  = MF.getSubtarget();
   const TargetMachine  = MF.getTarget();
+  Module  = *MF.getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::MRC, ARM::LDRi12);
+return;
+  }
 
   const GlobalValue *GV =
   cast((*MI->memoperands_begin())->getValue());
Index: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -4890,40 +4890,70 @@
   MachineBasicBlock  = *MI->getParent();
   DebugLoc DL = MI->getDebugLoc();
   Register Reg = MI->getOperand(0).getReg();
-  const GlobalValue *GV =
-  cast((*MI->memoperands_begin())->getValue());
-  bool IsIndirect = Subtarget.isGVIndirectSymbol(GV);
   MachineInstrBuilder MIB;
+  unsigned int Offset = 0;
 
-

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

2021-11-04 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb added a comment.

I would argue that the existing __builtin_addressof() should absorb this 
behavior, rather than adding a special builtin related to CFI.

As is documented for __builtin_addressof(), its intended use is in cases where 
the & operator may return something other than the address of the object, which 
is exactly the case we are dealing with here.




Comment at: clang/test/CodeGen/builtin-addressof-nocfi.c:18
+  // CHECK: call void @c(void ()* no_cfi @a)
+  c(__builtin_addressof_nocfi(a));
+  e();

samitolvanen wrote:
> nickdesaulniers wrote:
> > do we ever need the builtin address of a global that's NOT a function?
> > 
> > If so, we should add a test for that. If not, perhaps we don't need to 
> > waste space in every APValue?
> > do we ever need the builtin address of a global that's NOT a function?
> 
> I don't think so. This version does accept any global because it was modelled 
> after `__builtin_addressof()`, but we could look into limiting this only for 
> functions. Perhaps the built-in name should also be changed in that case?
> 
> > If so, we should add a test for that. If not, perhaps we don't need to 
> > waste space in every APValue?
> 
> What would be a better place for the flag? I think in Clang, changing this to 
> support only functions would probably just need some additional Sema checks.
> > do we ever need the builtin address of a global that's NOT a function?
> 
> I don't think so. This version does accept any global because it was modelled 
> after `__builtin_addressof()`, but we could look into limiting this only for 
> functions. Perhaps the built-in name should also be changed in that case?
> 

Why is that necessary? That will only make it harder to use for users that want 
to wrap this in another macro that may be used for arbitrary symbols.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108479/new/

https://reviews.llvm.org/D108479

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

2021-11-03 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb marked 2 inline comments as done.
ardb added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3190-3191
+  }
+  CmdArgs.push_back("-target-feature");
+  CmdArgs.push_back("+read-tp-hard");
+}

nickdesaulniers wrote:
> ardb wrote:
> > nickdesaulniers wrote:
> > > Isn't this redundant/set elsewhere?
> > No, it is not. The alternative is requiring the user to pass -mtp=cp15 when 
> > using the TLS stack protector, which is silly because it is implied.
> Right. Perhaps we could emit `diag::err_drv_argument_not_allowed_with` if the 
> user specified `-mtp=soft` with `-mstack-protector-guard=tls`.
> 
> It looks like a call to clang::driver::tools::arm::getReadTPMode() should 
> being able to tell us whether someone explicitly tried to use soft.
> Right. Perhaps we could emit `diag::err_drv_argument_not_allowed_with` if the 
> user specified `-mtp=soft` with `-mstack-protector-guard=tls`.
> 
> It looks like a call to clang::driver::tools::arm::getReadTPMode() should 
> being able to tell us whether someone explicitly tried to use soft.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112768/new/

https://reviews.llvm.org/D112768

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

2021-11-03 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb updated this revision to Diff 384564.
ardb added a comment.

- disallow -mtp=soft when TLS based stack protector is enabled


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112768/new/

https://reviews.llvm.org/D112768

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
  llvm/lib/Target/ARM/ARMInstrInfo.cpp
  llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
  llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
  llvm/test/CodeGen/ARM/stack-guard-tls.ll

Index: llvm/test/CodeGen/ARM/stack-guard-tls.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/stack-guard-tls.ll
@@ -0,0 +1,38 @@
+; RUN: split-file %s %t
+; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
+; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll
+; RUN: llc %t/a2.ll -mtriple=armv7-unknown-linux-gnueabihf -mattr=+read-tp-hard -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-SMALL %s
+; RUN: llc %t/a2.ll -mtriple=thumbv7-unknown-linux-gnueabihf -mattr=+read-tp-hard -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-SMALL %s
+; RUN: llc %t/b2.ll -mtriple=armv7-unknown-linux-gnueabihf -mattr=+read-tp-hard -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-LARGE %s
+; RUN: llc %t/b2.ll -mtriple=thumbv7-unknown-linux-gnueabihf -mattr=+read-tp-hard -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-LARGE %s
+
+;--- main.ll
+declare void @baz(i32*)
+
+define void @foo(i64 %t) sspstrong {
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+!llvm.module.flags = !{!1, !2}
+!1 = !{i32 2, !"stack-protector-guard", !"tls"}
+
+;--- a.ll
+!2 = !{i32 2, !"stack-protector-guard-offset", i32 1296}
+
+;--- b.ll
+!2 = !{i32 2, !"stack-protector-guard-offset", i32 4296}
+
+; CHECK: mrc p15, #0, [[REG1:r[0-9]+]], c13, c0, #3
+; CHECK-SMALL-NEXT: ldr{{(\.w)?}} [[REG1]], {{\[}}[[REG1]], #1296]
+; CHECK-LARGE-NEXT: add{{(\.w)?}} [[REG1]], [[REG1]], #4096
+; CHECK-LARGE-NEXT: ldr{{(\.w)?}} [[REG1]], {{\[}}[[REG1]], #200]
+; CHECK: bl baz
+; CHECK: mrc p15, #0, [[REG2:r[0-9]+]], c13, c0, #3
+; CHECK-SMALL-NEXT: ldr{{(\.w)?}} [[REG2]], {{\[}}[[REG2]], #1296]
+; CHECK-LARGE-NEXT: add{{(\.w)?}} [[REG2]], [[REG2]], #4096
+; CHECK-LARGE-NEXT: ldr{{(\.w)?}} [[REG2]], {{\[}}[[REG2]], #200]
Index: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
===
--- llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
+++ llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
@@ -250,6 +250,13 @@
 void Thumb2InstrInfo::expandLoadStackGuard(
 MachineBasicBlock::iterator MI) const {
   MachineFunction  = *MI->getParent()->getParent();
+  Module  = *MF.getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::t2MRC, ARM::t2LDRi12);
+return;
+  }
+
   const GlobalValue *GV =
   cast((*MI->memoperands_begin())->getValue());
 
Index: llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
===
--- llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
+++ llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
@@ -135,6 +135,11 @@
 MachineBasicBlock::iterator MI) const {
   MachineFunction  = *MI->getParent()->getParent();
   const TargetMachine  = MF.getTarget();
+  Module  = *MF.getFunction().getParent();
+
+  assert(M.getStackProtectorGuard() != "tls" &&
+ "TLS stack protector not supported for Thumb1 targets");
+
   if (TM.isPositionIndependent())
 expandLoadStackGuardBase(MI, ARM::tLDRLIT_ga_pcrel, ARM::tLDRi);
   else
Index: llvm/lib/Target/ARM/ARMInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMInstrInfo.cpp
@@ -95,6 +95,12 @@
   MachineFunction  = *MI->getParent()->getParent();
   const ARMSubtarget  = MF.getSubtarget();
   const TargetMachine  = MF.getTarget();
+  Module  = *MF.getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::MRC, ARM::LDRi12);
+return;
+  }
 
   const GlobalValue *GV =
   cast((*MI->memoperands_begin())->getValue());
Index: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -4891,40 +4891,70 @@
   MachineBasicBlock  = *MI->getParent();
   DebugLoc DL = MI->getDebugLoc();
   Register Reg = MI->getOperand(0).getReg();
-  const GlobalValue *GV =
-  cast((*MI->memoperands_begin())->getValue());
-  bool IsIndirect = Subtarget.isGVIndirectSymbol(GV);
   MachineInstrBuilder MIB;
+  unsigned int Offset = 0;
 
-  unsigned TargetFlags = ARMII::MO_NO_FLAG;
-  if (Subtarget.isTargetMachO()) {
-TargetFlags |=

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

2021-11-02 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3177-3179
+  if (!Args.hasArg(options::OPT_mstack_protector_guard_offset_EQ)) {
+D.Diag(diag::err_drv_ssp_missing_offset_argument)
+<< A->getOption().getName() << Value;

nickdesaulniers wrote:
> Does GCC require these flags to be paired (`-mstack-protector-guard=` and 
> `-mstack-protector-guard-offset`) ? Only for ARM and THUMB?  Doesn't the 
> offset only make sense for sysreg and tls, or global, too?
> 
> Seems to me like `0` would be a good default offset, if unspecified.
> 
> Perhaps this would good to check then warn on for //all// supported 
> architectures, and perhaps as another child patch? (or just default to `0` 
> and not require `-mstack-protector-offset=`).
> Does GCC require these flags to be paired (`-mstack-protector-guard=` and 
> `-mstack-protector-guard-offset`) ? Only for ARM and THUMB?  Doesn't the 
> offset only make sense for sysreg and tls, or global, too?
> 

GCC does not implement this for ARM yet. For AArch64, it requires all three 
options: guard, guard-reg and guard-offset.

> Seems to me like `0` would be a good default offset, if unspecified.
> 

The default is INT_MAX at the moment, and I wasn't sure if we can simply change 
that without breaking users.

> Perhaps this would good to check then warn on for //all// supported 
> architectures, and perhaps as another child patch? (or just default to `0` 
> and not require `-mstack-protector-offset=`).

Yes, I think it makes sense to check this for AAch64 as well. 



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3190-3191
+  }
+  CmdArgs.push_back("-target-feature");
+  CmdArgs.push_back("+read-tp-hard");
+}

nickdesaulniers wrote:
> Isn't this redundant/set elsewhere?
No, it is not. The alternative is requiring the user to pass -mtp=cp15 when 
using the TLS stack protector, which is silly because it is implied.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112768/new/

https://reviews.llvm.org/D112768

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113026: [ARM] reject -mtp=cp15 if target subarch does not support it

2021-11-02 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb marked 2 inline comments as done.
ardb added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:160-161
+if (ThreadPointer == ReadTPMode::Cp15 &&
+getARMSubArchVersionNumber(Triple) < 7 &&
+llvm::ARM::parseArch(Triple.getArchName()) != 
llvm::ARM::ArchKind::ARMV6T2) {
+  D.Diag(diag::err_target_unsupported_tp_hard) << Triple.getArchName();

nickdesaulniers wrote:
> The last two checks seems redundant (`getARMSubArchVersionNumber(Triple) < 7 
> && llvm::ARM::parseArch(Triple.getArchName()) != 
> llvm::ARM::ArchKind::ARMV6T2`).  If the `ArchKind` was `ARMV6T2` then 
> `getARMSubArchVersionNumber` should return `6`.
> 
> Or do we want to support ARMv6T2, in which case the patch description should 
> be edited:
> 
> > ensure that -mtp=cp15 is rejected for ARMv6T2 or earlier.
> 
> s/ARMv6T2 or earlier/ARMv6 (except ARMv6T2) or earlier/
> The last two checks seems redundant (`getARMSubArchVersionNumber(Triple) < 7 
> && llvm::ARM::parseArch(Triple.getArchName()) != 
> llvm::ARM::ArchKind::ARMV6T2`).  If the `ArchKind` was `ARMV6T2` then 
> `getARMSubArchVersionNumber` should return `6`.
> 
> Or do we want to support ARMv6T2, in which case the patch description should 
> be edited:
> 
> > ensure that -mtp=cp15 is rejected for ARMv6T2 or earlier.
> 
> s/ARMv6T2 or earlier/ARMv6 (except ARMv6T2) or earlier/

Yeah, the latter. ARMv6T2 is the first core that implements Thumb2, 




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113026/new/

https://reviews.llvm.org/D113026

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113026: [ARM] reject -mtp=cp15 if target subarch does not support it

2021-11-02 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb updated this revision to Diff 384175.
ardb edited the summary of this revision.
ardb added a comment.

Fix test issue and add more test cases


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113026/new/

https://reviews.llvm.org/D113026

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/clang-translation.c


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -110,15 +110,27 @@
 // ARMV5E: "-cc1"
 // ARMV5E: "-target-cpu" "arm1022e"
 
-// RUN: %clang -target arm-linux -mtp=cp15 -### -S %s -arch armv7 2>&1 | \
+// RUN: %clang -target armv7-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER-HARD %s
 // ARMv7_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
 
-// RUN: %clang -target arm-linux -mtp=soft -### -S %s -arch armv7 2>&1 | \
+// RUN: %clang -target armv6t2-linux -mtp=cp15 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv6T2_THREAD_POINTER-HARD %s
+// ARMv6T2_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
+
+// RUN: %clang -target armv5t-linux -mtp=cp15 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv5_THREAD_POINTER_UNSUPP %s
+// ARMv5_THREAD_POINTER_UNSUPP: hardware TLS register is not supported for the 
armv5 sub-architecture
+
+// RUN: %clang -target thumbv6-linux -mtp=cp15 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv6_THREAD_POINTER_UNSUPP %s
+// ARMv6_THREAD_POINTER_UNSUPP: hardware TLS register is not supported for the 
armv6 sub-architecture
+
+// RUN: %clang -target armv7-linux -mtp=soft -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_SOFT %s
 // ARMv7_THREAD_POINTER_SOFT-NOT: "-target-feature" "+read-tp-hard"
 
-// RUN: %clang -target arm-linux -### -S %s -arch armv7 2>&1 | \
+// RUN: %clang -target armv7-linux -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_NON %s
 // ARMv7_THREAD_POINTER_NON-NOT: "-target-feature" "+read-tp-hard"
 
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -53,7 +53,8 @@
 const llvm::opt::ArgList );
 void setFloatABIInTriple(const Driver , const llvm::opt::ArgList ,
  llvm::Triple );
-ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList );
+ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList ,
+ const llvm::Triple );
 void setArchNameInTriple(const Driver , const llvm::opt::ArgList ,
  types::ID InputType, llvm::Triple );
 
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -148,13 +148,21 @@
 }
 
 // Select mode for reading thread pointer (-mtp=soft/cp15).
-arm::ReadTPMode arm::getReadTPMode(const Driver , const ArgList ) {
+arm::ReadTPMode arm::getReadTPMode(const Driver , const ArgList ,
+   const llvm::Triple ) {
   if (Arg *A = Args.getLastArg(options::OPT_mtp_mode_EQ)) {
 arm::ReadTPMode ThreadPointer =
 llvm::StringSwitch(A->getValue())
 .Case("cp15", ReadTPMode::Cp15)
 .Case("soft", ReadTPMode::Soft)
 .Default(ReadTPMode::Invalid);
+if (ThreadPointer == ReadTPMode::Cp15 &&
+getARMSubArchVersionNumber(Triple) < 7 &&
+llvm::ARM::parseArch(Triple.getArchName()) !=
+llvm::ARM::ArchKind::ARMV6T2) {
+  D.Diag(diag::err_target_unsupported_tp_hard) << Triple.getArchName();
+  return ReadTPMode::Invalid;
+}
 if (ThreadPointer != ReadTPMode::Invalid)
   return ThreadPointer;
 if (StringRef(A->getValue()).empty())
@@ -422,7 +430,7 @@
   bool KernelOrKext =
   Args.hasArg(options::OPT_mkernel, options::OPT_fapple_kext);
   arm::FloatABI ABI = arm::getARMFloatABI(D, Triple, Args);
-  arm::ReadTPMode ThreadPointer = arm::getReadTPMode(D, Args);
+  arm::ReadTPMode ThreadPointer = arm::getReadTPMode(D, Args, Triple);
   llvm::Optional> WaCPU, WaFPU, WaHDiv,
   WaArch;
 


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -110,15 +110,27 @@
 // ARMV5E: "-cc1"
 // ARMV5E: "-target-cpu" "arm1022e"
 
-// RUN: %clang -target arm-linux -mtp=cp15 -### -S %s -arch armv7 2>&1 | \
+// RUN: %clang -target armv7-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER-HARD %s
 // ARMv7_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
 
-// RUN: %clang -target arm-linux -mtp=soft -###

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

2021-11-02 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb updated this revision to Diff 384154.
ardb added a comment.

- fix failure in newly added LLVM test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112768/new/

https://reviews.llvm.org/D112768

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
  llvm/lib/Target/ARM/ARMInstrInfo.cpp
  llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
  llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
  llvm/test/CodeGen/ARM/stack-guard-tls.ll

Index: llvm/test/CodeGen/ARM/stack-guard-tls.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/stack-guard-tls.ll
@@ -0,0 +1,38 @@
+; RUN: split-file %s %t
+; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
+; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll
+; RUN: llc %t/a2.ll -mtriple=armv7-unknown-linux-gnueabihf -mattr=+read-tp-hard -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-SMALL %s
+; RUN: llc %t/a2.ll -mtriple=thumbv7-unknown-linux-gnueabihf -mattr=+read-tp-hard -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-SMALL %s
+; RUN: llc %t/b2.ll -mtriple=armv7-unknown-linux-gnueabihf -mattr=+read-tp-hard -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-LARGE %s
+; RUN: llc %t/b2.ll -mtriple=thumbv7-unknown-linux-gnueabihf -mattr=+read-tp-hard -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-LARGE %s
+
+;--- main.ll
+declare void @baz(i32*)
+
+define void @foo(i64 %t) sspstrong {
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+!llvm.module.flags = !{!1, !2}
+!1 = !{i32 2, !"stack-protector-guard", !"tls"}
+
+;--- a.ll
+!2 = !{i32 2, !"stack-protector-guard-offset", i32 1296}
+
+;--- b.ll
+!2 = !{i32 2, !"stack-protector-guard-offset", i32 4296}
+
+; CHECK: mrc p15, #0, [[REG1:r[0-9]+]], c13, c0, #3
+; CHECK-SMALL-NEXT: ldr{{(\.w)?}} [[REG1]], {{\[}}[[REG1]], #1296]
+; CHECK-LARGE-NEXT: add{{(\.w)?}} [[REG1]], [[REG1]], #4096
+; CHECK-LARGE-NEXT: ldr{{(\.w)?}} [[REG1]], {{\[}}[[REG1]], #200]
+; CHECK: bl baz
+; CHECK: mrc p15, #0, [[REG2:r[0-9]+]], c13, c0, #3
+; CHECK-SMALL-NEXT: ldr{{(\.w)?}} [[REG2]], {{\[}}[[REG2]], #1296]
+; CHECK-LARGE-NEXT: add{{(\.w)?}} [[REG2]], [[REG2]], #4096
+; CHECK-LARGE-NEXT: ldr{{(\.w)?}} [[REG2]], {{\[}}[[REG2]], #200]
Index: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
===
--- llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
+++ llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
@@ -250,6 +250,13 @@
 void Thumb2InstrInfo::expandLoadStackGuard(
 MachineBasicBlock::iterator MI) const {
   MachineFunction  = *MI->getParent()->getParent();
+  Module  = *MF.getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::t2MRC, ARM::t2LDRi12);
+return;
+  }
+
   const GlobalValue *GV =
   cast((*MI->memoperands_begin())->getValue());
 
Index: llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
===
--- llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
+++ llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
@@ -135,6 +135,11 @@
 MachineBasicBlock::iterator MI) const {
   MachineFunction  = *MI->getParent()->getParent();
   const TargetMachine  = MF.getTarget();
+  Module  = *MF.getFunction().getParent();
+
+  assert(M.getStackProtectorGuard() != "tls" &&
+ "TLS stack protector not supported for Thumb1 targets");
+
   if (TM.isPositionIndependent())
 expandLoadStackGuardBase(MI, ARM::tLDRLIT_ga_pcrel, ARM::tLDRi);
   else
Index: llvm/lib/Target/ARM/ARMInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMInstrInfo.cpp
@@ -95,6 +95,12 @@
   MachineFunction  = *MI->getParent()->getParent();
   const ARMSubtarget  = MF.getSubtarget();
   const TargetMachine  = MF.getTarget();
+  Module  = *MF.getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::MRC, ARM::LDRi12);
+return;
+  }
 
   const GlobalValue *GV =
   cast((*MI->memoperands_begin())->getValue());
Index: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -4891,40 +4891,70 @@
   MachineBasicBlock  = *MI->getParent();
   DebugLoc DL = MI->getDebugLoc();
   Register Reg = MI->getOperand(0).getReg();
-  const GlobalValue *GV =
-  cast((*MI->memoperands_begin())->getValue());
-  bool IsIndirect = Subtarget.isGVIndirectSymbol(GV);
   MachineInstrBuilder MIB;
+  unsigned int Offset = 0;
 
-  unsigned TargetFlags = ARMII::MO_NO_FLAG;
-  if (Subtarget.isTargetMachO()) {
-TargetFlags |= ARMII::MO_NONLAZY;
-  } else

[PATCH] D113026: [ARM] reject -mtp=cp15 if target subarch does not support it

2021-11-02 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb created this revision.
ardb added reviewers: nickdesaulniers, peter.smith, rengolin, kees, ostannard.
Herald added a subscriber: kristof.beyls.
ardb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently, we permit -mtp=cp15 even for targets that don't implement the TLS 
register. When building for ARMv6 or earlier, this means we emit instructions 
that will UNDEF at runtime. For Thumb1, passing -mtp=cp15 will trigger an 
assert in the backend.

So let's add some diagnostics to ensure that -mtp=cp15 is rejected for ARMv6T2 
or earlier.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113026

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/clang-translation.c


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -110,15 +110,19 @@
 // ARMV5E: "-cc1"
 // ARMV5E: "-target-cpu" "arm1022e"
 
-// RUN: %clang -target arm-linux -mtp=cp15 -### -S %s -arch armv7 2>&1 | \
+// RUN: %clang -target armv7-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER-HARD %s
 // ARMv7_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
 
-// RUN: %clang -target arm-linux -mtp=soft -### -S %s -arch armv7 2>&1 | \
+// RUN: %clang -target armv5t-linux -mtp=cp15 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_UNSUPPORTED %s
+// ARMv7_THREAD_POINTER_UNSUPPORTED: error: hardware TLS register is not 
supported for the armv5 sub-architecture
+
+// RUN: %clang -target armv7-linux -mtp=soft -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_SOFT %s
 // ARMv7_THREAD_POINTER_SOFT-NOT: "-target-feature" "+read-tp-hard"
 
-// RUN: %clang -target arm-linux -### -S %s -arch armv7 2>&1 | \
+// RUN: %clang -target armv7-linux -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_NON %s
 // ARMv7_THREAD_POINTER_NON-NOT: "-target-feature" "+read-tp-hard"
 
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -53,7 +53,8 @@
 const llvm::opt::ArgList );
 void setFloatABIInTriple(const Driver , const llvm::opt::ArgList ,
  llvm::Triple );
-ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList );
+ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList ,
+ const llvm::Triple );
 void setArchNameInTriple(const Driver , const llvm::opt::ArgList ,
  types::ID InputType, llvm::Triple );
 
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -148,13 +148,20 @@
 }
 
 // Select mode for reading thread pointer (-mtp=soft/cp15).
-arm::ReadTPMode arm::getReadTPMode(const Driver , const ArgList ) {
+arm::ReadTPMode arm::getReadTPMode(const Driver , const ArgList ,
+   const llvm::Triple ) {
   if (Arg *A = Args.getLastArg(options::OPT_mtp_mode_EQ)) {
 arm::ReadTPMode ThreadPointer =
 llvm::StringSwitch(A->getValue())
 .Case("cp15", ReadTPMode::Cp15)
 .Case("soft", ReadTPMode::Soft)
 .Default(ReadTPMode::Invalid);
+if (ThreadPointer == ReadTPMode::Cp15 &&
+getARMSubArchVersionNumber(Triple) < 7 &&
+llvm::ARM::parseArch(Triple.getArchName()) != 
llvm::ARM::ArchKind::ARMV6T2) {
+  D.Diag(diag::err_target_unsupported_tp_hard) << Triple.getArchName();
+  return ReadTPMode::Invalid;
+}
 if (ThreadPointer != ReadTPMode::Invalid)
   return ThreadPointer;
 if (StringRef(A->getValue()).empty())
@@ -422,7 +429,7 @@
   bool KernelOrKext =
   Args.hasArg(options::OPT_mkernel, options::OPT_fapple_kext);
   arm::FloatABI ABI = arm::getARMFloatABI(D, Triple, Args);
-  arm::ReadTPMode ThreadPointer = arm::getReadTPMode(D, Args);
+  arm::ReadTPMode ThreadPointer = arm::getReadTPMode(D, Args, Triple);
   llvm::Optional> WaCPU, WaFPU, WaHDiv,
   WaArch;
 


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -110,15 +110,19 @@
 // ARMV5E: "-cc1"
 // ARMV5E: "-target-cpu" "arm1022e"
 
-// RUN: %clang -target arm-linux -mtp=cp15 -### -S %s -arch armv7 2>&1 | \
+// RUN: %clang -target armv7-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER-HARD %s
 // ARMv7_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
 
-// RUN: %clang -target arm-linux -mtp=soft

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

2021-11-02 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb updated this revision to Diff 384116.
ardb added a comment.

- add diagnostics to the frontend and asserts to the backend to ensure that the 
TLS stack protector is only used on target subarchs that implement the hardware 
TLS register to begin with
- ensure that the offset parameter is not omitted, as the default is INT_MAX 
which is out of bounds


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112768/new/

https://reviews.llvm.org/D112768

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
  llvm/lib/Target/ARM/ARMInstrInfo.cpp
  llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
  llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
  llvm/test/CodeGen/ARM/stack-guard-tls.ll

Index: llvm/test/CodeGen/ARM/stack-guard-tls.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/stack-guard-tls.ll
@@ -0,0 +1,38 @@
+; RUN: split-file %s %t
+; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
+; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll
+; RUN: llc %t/a2.ll -mtriple=armv7-unknown-linux-gnueabihf -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-SMALL %s
+; RUN: llc %t/a2.ll -mtriple=thumbv7-unknown-linux-gnueabihf -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-SMALL %s
+; RUN: llc %t/b2.ll -mtriple=armv7-unknown-linux-gnueabihf -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-LARGE %s
+; RUN: llc %t/b2.ll -mtriple=thumbv7-unknown-linux-gnueabihf -o - | \
+; RUN: FileCheck --check-prefixes=CHECK,CHECK-LARGE %s
+
+;--- main.ll
+declare void @baz(i32*)
+
+define void @foo(i64 %t) sspstrong {
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+!llvm.module.flags = !{!1, !2}
+!1 = !{i32 2, !"stack-protector-guard", !"tls"}
+
+;--- a.ll
+!2 = !{i32 2, !"stack-protector-guard-offset", i32 1296}
+
+;--- b.ll
+!2 = !{i32 2, !"stack-protector-guard-offset", i32 4296}
+
+; CHECK: mrc p15, #0, [[REG1:r[0-9]+]], c13, c0, #3
+; CHECK-SMALL-NEXT: ldr{{(\.w)?}} [[REG1]], {{\[}}[[REG1]], #1296]
+; CHECK-LARGE-NEXT: add{{(\.w)?}} [[REG1]], [[REG1]], #4096
+; CHECK-LARGE-NEXT: ldr{{(\.w)?}} [[REG1]], {{\[}}[[REG1]], #200]
+; CHECK: bl baz
+; CHECK: mrc p15, #0, [[REG2:r[0-9]+]], c13, c0, #3
+; CHECK-SMALL-NEXT: ldr{{(\.w)?}} [[REG2]], {{\[}}[[REG2]], #1296]
+; CHECK-LARGE-NEXT: add{{(\.w)?}} [[REG2]], [[REG2]], #4096
+; CHECK-LARGE-NEXT: ldr{{(\.w)?}} [[REG2]], {{\[}}[[REG2]], #200]
Index: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
===
--- llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
+++ llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
@@ -250,6 +250,13 @@
 void Thumb2InstrInfo::expandLoadStackGuard(
 MachineBasicBlock::iterator MI) const {
   MachineFunction  = *MI->getParent()->getParent();
+  Module  = *MF.getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::t2MRC, ARM::t2LDRi12);
+return;
+  }
+
   const GlobalValue *GV =
   cast((*MI->memoperands_begin())->getValue());
 
Index: llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
===
--- llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
+++ llvm/lib/Target/ARM/Thumb1InstrInfo.cpp
@@ -135,6 +135,11 @@
 MachineBasicBlock::iterator MI) const {
   MachineFunction  = *MI->getParent()->getParent();
   const TargetMachine  = MF.getTarget();
+  Module  = *MF.getFunction().getParent();
+
+  assert(M.getStackProtectorGuard() != "tls" &&
+ "TLS stack protector not supported for Thumb1 targets");
+
   if (TM.isPositionIndependent())
 expandLoadStackGuardBase(MI, ARM::tLDRLIT_ga_pcrel, ARM::tLDRi);
   else
Index: llvm/lib/Target/ARM/ARMInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMInstrInfo.cpp
@@ -95,6 +95,12 @@
   MachineFunction  = *MI->getParent()->getParent();
   const ARMSubtarget  = MF.getSubtarget();
   const TargetMachine  = MF.getTarget();
+  Module  = *MF.getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::MRC, ARM::LDRi12);
+return;
+  }
 
   const GlobalValue *GV =
   cast((*MI->memoperands_begin())->getValue());
Index: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -4891,40 +4891,70 @@
   MachineBasicBlock  = *MI->getParent();
   DebugLoc DL = MI->getDebugLoc();
   Register Reg = MI->getOperand(0).getReg();
-  const GlobalValue *GV =
-  cast((*MI->memoperands_begin())->getValue());
-  bool IsIndirect = Subtarget.isGVIndirectSymbol(GV);
   MachineInstrBuilder

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

2021-10-29 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb added inline comments.



Comment at: llvm/lib/Target/ARM/ARMInstrInfo.cpp:102
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::MRC, ARM::LDRi12);
+return;

nickdesaulniers wrote:
> ardb wrote:
> > nickdesaulniers wrote:
> > > do we have to worry about soft tp, ie. `__aeabi_read_tp` vs `mrc`?
> > I think we should only allow this hard TP is supported in the first place. 
> > I'll add a check somewhere.
> > Calling a helper in both the prologue and the epilogue for emulated TLS 
> > seems a bit too heavyweight to me, and the Linux kernel will not use it in 
> > that way anyway.
> I agree. Let's add a test that this only works with the 
> `-mattr=+read-tp-hard` llc flag or `"target-features"="+read-tp-hard"` 
> function attribute (one or the other, I don't think we need to exhaustively 
> test both). But it would be good to verify what happens when `+read-tp-hard` 
> is not set, in case someone else cares to add soft tp support here in the 
> future.  (They may never, it's more so to highlight whether such a change 
> would be intentional).
> 
> `ARMSubtarget::isReadTPHard()` should be able to help us differentiate this 
> case.
> I agree. Let's add a test that this only works with the 
> `-mattr=+read-tp-hard` llc flag or `"target-features"="+read-tp-hard"` 
> function attribute (one or the other, I don't think we need to exhaustively 
> test both). But it would be good to verify what happens when `+read-tp-hard` 
> is not set, in case someone else cares to add soft tp support here in the 
> future.  (They may never, it's more so to highlight whether such a change 
> would be intentional).
> 
> `ARMSubtarget::isReadTPHard()` should be able to help us differentiate this 
> case.

I could not figure out how to check this at command line parsing time. If we 
check it later, we basically crash the compiler, right? Not very elegant ...





Comment at: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp:250
 
 void Thumb2InstrInfo::expandLoadStackGuard(
 MachineBasicBlock::iterator MI) const {

nickdesaulniers wrote:
> ardb wrote:
> > nickdesaulniers wrote:
> > > what about `Thumb1InstrInfo::expandLoadStackGuard`? Do we have `mrc` 
> > > available in Thumb[1] as well?
> > No Thumb1 does not have an encoding for MRC so we can ignore it here. It 
> > does mean we should not allow this feature to be turned out for such 
> > targets.
> Let's add a test for such case.
> Let's add a test for such case.

Same problem: I could not figure out how to decide early enough whether we are 
targetting Thumb1 or Thumb2/ARM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112768/new/

https://reviews.llvm.org/D112768

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

2021-10-29 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb updated this revision to Diff 383390.
ardb edited the summary of this revision.
ardb added a comment.

- split off LOAD_STACK_GUARD conversion
- deal with guard offsets >= 4096 bytes
- reject offsets < 0 or >= 1 MiB
- add backend test to check that the MRC/LDR sequence is emitted twice


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112768/new/

https://reviews.llvm.org/D112768

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
  llvm/lib/Target/ARM/ARMInstrInfo.cpp
  llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
  llvm/test/CodeGen/ARM/stack-guard-tls.ll

Index: llvm/test/CodeGen/ARM/stack-guard-tls.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/stack-guard-tls.ll
@@ -0,0 +1,38 @@
+; RUN: split-file %s %t
+; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
+; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll
+; RUN: llc %t/a2.ll -mtriple=armv7-unknown-linux-gnueabihf -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-SMALL %s
+; RUN: llc %t/a2.ll -mtriple=thumbv7-unknown-linux-gnueabihf -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-SMALL %s
+; RUN: llc %t/b2.ll -mtriple=armv7-unknown-linux-gnueabihf -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-LARGE %s
+; RUN: llc %t/b2.ll -mtriple=thumbv7-unknown-linux-gnueabihf -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-LARGE %s
+
+;--- main.ll
+declare void @baz(i32*)
+
+define void @foo(i64 %t) sspstrong {
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+!llvm.module.flags = !{!1, !2}
+!1 = !{i32 2, !"stack-protector-guard", !"tls"}
+
+;--- a.ll
+!2 = !{i32 2, !"stack-protector-guard-offset", i32 1296}
+
+;--- b.ll
+!2 = !{i32 2, !"stack-protector-guard-offset", i32 4296}
+
+; CHECK: mrc p15, #0, [[REG1:r[0-9]+]], c13, c0, #3
+; CHECK-SMALL-NEXT: ldr{{(\.w)?}} [[REG1]], {{\[}}[[REG1]], #1296]
+; CHECK-LARGE-NEXT: add{{(\.w)?}} [[REG1]], [[REG1]], #4096
+; CHECK-LARGE-NEXT: ldr{{(\.w)?}} [[REG1]], {{\[}}[[REG1]], #200]
+; CHECK: bl baz
+; CHECK: mrc p15, #0, [[REG2:r[0-9]+]], c13, c0, #3
+; CHECK-SMALL-NEXT: ldr{{(\.w)?}} [[REG2]], {{\[}}[[REG2]], #1296]
+; CHECK-LARGE-NEXT: add{{(\.w)?}} [[REG2]], [[REG2]], #4096
+; CHECK-LARGE-NEXT: ldr{{(\.w)?}} [[REG2]], {{\[}}[[REG2]], #200]
Index: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
===
--- llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
+++ llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
@@ -250,6 +250,14 @@
 void Thumb2InstrInfo::expandLoadStackGuard(
 MachineBasicBlock::iterator MI) const {
   MachineFunction  = *MI->getParent()->getParent();
+  MachineBasicBlock  = *MI->getParent();
+  Module  = *MBB.getParent()->getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::t2MRC, ARM::t2LDRi12);
+return;
+  }
+
   const GlobalValue *GV =
   cast((*MI->memoperands_begin())->getValue());
 
Index: llvm/lib/Target/ARM/ARMInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMInstrInfo.cpp
@@ -95,6 +95,13 @@
   MachineFunction  = *MI->getParent()->getParent();
   const ARMSubtarget  = MF.getSubtarget();
   const TargetMachine  = MF.getTarget();
+  MachineBasicBlock  = *MI->getParent();
+  Module  = *MBB.getParent()->getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::MRC, ARM::LDRi12);
+return;
+  }
 
   const GlobalValue *GV =
   cast((*MI->memoperands_begin())->getValue());
@@ -117,7 +124,6 @@
 return;
   }
 
-  MachineBasicBlock  = *MI->getParent();
   DebugLoc DL = MI->getDebugLoc();
   Register Reg = MI->getOperand(0).getReg();
   MachineInstrBuilder MIB;
Index: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -4891,40 +4891,67 @@
   MachineBasicBlock  = *MI->getParent();
   DebugLoc DL = MI->getDebugLoc();
   Register Reg = MI->getOperand(0).getReg();
-  const GlobalValue *GV =
-  cast((*MI->memoperands_begin())->getValue());
-  bool IsIndirect = Subtarget.isGVIndirectSymbol(GV);
   MachineInstrBuilder MIB;
+  unsigned int Offset = 0;
 
-  unsigned TargetFlags = ARMII::MO_NO_FLAG;
-  if (Subtarget.isTargetMachO()) {
-TargetFlags |= ARMII::MO_NONLAZY;
-  } else if (Subtarget.isTargetCOFF()) {
-if (GV->hasDLLImportStorageClass())
-  TargetFlags |= ARMII::MO_DLLIMPORT;
-else if (IsIndirect)
-  TargetFlags |= ARMII::MO_COFFSTUB;
-  } else if (Subtarget.isGVInGOT(GV)) {
-TargetFlags |= ARMII::MO_GOT;
-  }
+  if (LoadImmOpc == ARM::MRC || LoadImmOpc == ARM::t2MRC) {
+

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

2021-10-29 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb added a comment.

I have split off the LOAD_STACK_GUARD changes into

  [ARM] implement LOAD_STACK_GUARD for remaining targets




Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:4900
+.addImm(15)
+.addImm(0)
+.addImm(13)

nickdesaulniers wrote:
> I think we want to use `Reg` in this instruction, in order to load into the 
> specified destination?
We use Reg, no? MRC does not take reg operands, but I pass it as the target 
register.



Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:4904
+.addImm(3)
+.add(predOps(ARMCC::AL));
 

nickdesaulniers wrote:
> do we need to add `al`?
It appears so.



Comment at: llvm/lib/Target/ARM/ARMInstrInfo.cpp:102
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::MRC, ARM::LDRi12);
+return;

nickdesaulniers wrote:
> do we have to worry about soft tp, ie. `__aeabi_read_tp` vs `mrc`?
I think we should only allow this hard TP is supported in the first place. I'll 
add a check somewhere.
Calling a helper in both the prologue and the epilogue for emulated TLS seems a 
bit too heavyweight to me, and the Linux kernel will not use it in that way 
anyway.



Comment at: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp:250
 
 void Thumb2InstrInfo::expandLoadStackGuard(
 MachineBasicBlock::iterator MI) const {

nickdesaulniers wrote:
> what about `Thumb1InstrInfo::expandLoadStackGuard`? Do we have `mrc` 
> available in Thumb[1] as well?
No Thumb1 does not have an encoding for MRC so we can ignore it here. It does 
mean we should not allow this feature to be turned out for such targets.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112768/new/

https://reviews.llvm.org/D112768

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

2021-10-28 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb created this revision.
ardb added reviewers: nickdesaulniers, peter.smith, nathanchance, kees.
Herald added subscribers: hiraditya, kristof.beyls.
ardb requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Implement support for loading the stack canary from a memory location held in 
the TLS register, with an optional offset applied. This is used by the Linux
kernel to implement per-task stack canaries, which is impossible on SMP systems
when using a global variable for the stack canary.

This also involves implementing LOAD_STACK_GUARD for other ARM targets than 
Mach-O.

Suggestions for how to test this and where to insert the test code into the 
tree are kindly welcomed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112768

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMInstrInfo.cpp
  llvm/lib/Target/ARM/Thumb2InstrInfo.cpp

Index: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
===
--- llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
+++ llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
@@ -250,7 +250,20 @@
 void Thumb2InstrInfo::expandLoadStackGuard(
 MachineBasicBlock::iterator MI) const {
   MachineFunction  = *MI->getParent()->getParent();
-  if (MF.getTarget().isPositionIndependent())
+  MachineBasicBlock  = *MI->getParent();
+  Module  = *MBB.getParent()->getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::t2MRC, ARM::t2LDRi12);
+return;
+  }
+
+  const GlobalValue *GV =
+cast((*MI->memoperands_begin())->getValue());
+
+  if (MF.getSubtarget().isGVInGOT(GV))
+expandLoadStackGuardBase(MI, ARM::tLDRLIT_ga_pcrel, ARM::t2LDRi12);
+  else if (MF.getTarget().isPositionIndependent())
 expandLoadStackGuardBase(MI, ARM::t2MOV_ga_pcrel, ARM::t2LDRi12);
   else
 expandLoadStackGuardBase(MI, ARM::t2MOVi32imm, ARM::t2LDRi12);
Index: llvm/lib/Target/ARM/ARMInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMInstrInfo.cpp
@@ -95,8 +95,18 @@
   MachineFunction  = *MI->getParent()->getParent();
   const ARMSubtarget  = MF.getSubtarget();
   const TargetMachine  = MF.getTarget();
+  MachineBasicBlock  = *MI->getParent();
+  Module  = *MBB.getParent()->getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::MRC, ARM::LDRi12);
+return;
+  }
 
-  if (!Subtarget.useMovt()) {
+  const GlobalValue *GV =
+  cast((*MI->memoperands_begin())->getValue());
+
+  if (!Subtarget.useMovt() || Subtarget.isGVInGOT(GV)) {
 if (TM.isPositionIndependent())
   expandLoadStackGuardBase(MI, ARM::LDRLIT_ga_pcrel, ARM::LDRi12);
 else
@@ -109,15 +119,11 @@
 return;
   }
 
-  const GlobalValue *GV =
-  cast((*MI->memoperands_begin())->getValue());
-
   if (!Subtarget.isGVIndirectSymbol(GV)) {
 expandLoadStackGuardBase(MI, ARM::MOV_ga_pcrel, ARM::LDRi12);
 return;
   }
 
-  MachineBasicBlock  = *MI->getParent();
   DebugLoc DL = MI->getDebugLoc();
   Register Reg = MI->getOperand(0).getReg();
   MachineInstrBuilder MIB;
Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
===
--- llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -20687,9 +20687,8 @@
   return InsertFencesForAtomic;
 }
 
-// This has so far only been implemented for MachO.
 bool ARMTargetLowering::useLoadStackGuardNode() const {
-  return Subtarget->isTargetMachO();
+  return true;
 }
 
 void ARMTargetLowering::insertSSPDeclarations(Module ) const {
Index: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -1682,8 +1682,6 @@
 
 bool ARMBaseInstrInfo::expandPostRAPseudo(MachineInstr ) const {
   if (MI.getOpcode() == TargetOpcode::LOAD_STACK_GUARD) {
-assert(getSubtarget().getTargetTriple().isOSBinFormatMachO() &&
-   "LOAD_STACK_GUARD currently supported only for MachO.");
 expandLoadStackGuard(MI);
 MI.getParent()->erase(MI);
 return true;
@@ -4884,8 +4882,6 @@
   return true;
 }
 
-// LoadStackGuard has so far only been implemented for MachO. Different code
-// sequence is needed for other targets.
 void ARMBaseInstrInfo::expandLoadStackGuardBase(MachineBasicBlock::iterator MI,
 unsigned LoadImmOpc,
 unsigned LoadOpc) const {
@@ -4895,27 +4891,48 @@
   MachineBasicBlock  = *MI->getParent();
   DebugLoc DL = MI->getDebugLoc();
   Register Reg =

[PATCH] D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed

2021-10-27 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb added a comment.

Thanks all. Could someone with commit access please merge this? Thanks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112600/new/

https://reviews.llvm.org/D112600

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed

2021-10-27 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb updated this revision to Diff 382839.
ardb added a comment.

Add another test suggested by Nick.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112600/new/

https://reviews.llvm.org/D112600

Files:
  llvm/lib/Target/ARM/ARMInstrThumb.td
  llvm/lib/Target/ARM/ARMInstrThumb2.td
  llvm/test/CodeGen/ARM/readtp.ll
  llvm/test/CodeGen/ARM/thread_pointer.ll


Index: llvm/test/CodeGen/ARM/thread_pointer.ll
===
--- llvm/test/CodeGen/ARM/thread_pointer.ll
+++ llvm/test/CodeGen/ARM/thread_pointer.ll
@@ -1,4 +1,7 @@
-; RUN: llc -mtriple arm-linux-gnueabi -filetype asm -o - %s | FileCheck %s
+; RUN: llc -mtriple arm-linux-gnueabi -o - %s | FileCheck %s 
-check-prefix=CHECK-SOFT
+; RUN: llc -mtriple arm-linux-gnueabi -mattr=+read-tp-hard -o - %s | FileCheck 
%s -check-prefix=CHECK-HARD
+; RUN: llc -mtriple thumbv7-linux-gnueabi -o - %s | FileCheck %s 
-check-prefix=CHECK-SOFT
+; RUN: llc -mtriple thumbv7-linux-gnueabi -mattr=+read-tp-hard -o - %s | 
FileCheck %s -check-prefix=CHECK-HARD
 
 declare i8* @llvm.thread.pointer()
 
@@ -8,5 +11,6 @@
   ret i8* %tmp1
 }
 
-; CHECK: bl __aeabi_read_tp
+; CHECK-SOFT: bl __aeabi_read_tp
+; CHECK-HARD: mrc p15, #0, {{r[0-9]+}}, c13, c0, #3
 
Index: llvm/test/CodeGen/ARM/readtp.ll
===
--- llvm/test/CodeGen/ARM/readtp.ll
+++ llvm/test/CodeGen/ARM/readtp.ll
@@ -1,5 +1,7 @@
 ; RUN: llc -mtriple=armeb-linux-gnueabihf -O2 -mattr=+read-tp-hard %s -o - | 
FileCheck %s -check-prefix=CHECK-HARD
 ; RUN: llc -mtriple=armeb-linux-gnueabihf -O2 %s -o - | FileCheck %s 
-check-prefix=CHECK-SOFT
+; RUN: llc -mtriple=thumbv7-linux-gnueabihf -O2 -mattr=+read-tp-hard %s -o - | 
FileCheck %s -check-prefix=CHECK-HARD
+; RUN: llc -mtriple=thumbv7-linux-gnueabihf -O2 %s -o - | FileCheck %s 
-check-prefix=CHECK-SOFT
 
 
 ; __thread int counter;
Index: llvm/lib/Target/ARM/ARMInstrThumb2.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb2.td
+++ llvm/lib/Target/ARM/ARMInstrThumb2.td
@@ -4671,6 +4671,9 @@
 }
 
 
+// Reading thread pointer from coprocessor register
+def : T2Pat<(ARMthread_pointer), (t2MRC 15, 0, 13, 0, 3)>,
+  Requires<[IsThumb2, IsReadTPHard]>;
 
 
//===--===//
 // ARMv8.1 Privilege Access Never extension
Index: llvm/lib/Target/ARM/ARMInstrThumb.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb.td
+++ llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -1520,6 +1520,7 @@
 let isCall = 1, Defs = [R0, R12, LR, CPSR], Uses = [SP] in
 def tTPsoft : tPseudoInst<(outs), (ins), 4, IIC_Br,
   [(set R0, ARMthread_pointer)]>,
+  Requires<[IsThumb, IsReadTPSoft]>,
   Sched<[WriteBr]>;
 
 
//===--===//


Index: llvm/test/CodeGen/ARM/thread_pointer.ll
===
--- llvm/test/CodeGen/ARM/thread_pointer.ll
+++ llvm/test/CodeGen/ARM/thread_pointer.ll
@@ -1,4 +1,7 @@
-; RUN: llc -mtriple arm-linux-gnueabi -filetype asm -o - %s | FileCheck %s
+; RUN: llc -mtriple arm-linux-gnueabi -o - %s | FileCheck %s -check-prefix=CHECK-SOFT
+; RUN: llc -mtriple arm-linux-gnueabi -mattr=+read-tp-hard -o - %s | FileCheck %s -check-prefix=CHECK-HARD
+; RUN: llc -mtriple thumbv7-linux-gnueabi -o - %s | FileCheck %s -check-prefix=CHECK-SOFT
+; RUN: llc -mtriple thumbv7-linux-gnueabi -mattr=+read-tp-hard -o - %s | FileCheck %s -check-prefix=CHECK-HARD
 
 declare i8* @llvm.thread.pointer()
 
@@ -8,5 +11,6 @@
   ret i8* %tmp1
 }
 
-; CHECK: bl __aeabi_read_tp
+; CHECK-SOFT: bl __aeabi_read_tp
+; CHECK-HARD: mrc p15, #0, {{r[0-9]+}}, c13, c0, #3
 
Index: llvm/test/CodeGen/ARM/readtp.ll
===
--- llvm/test/CodeGen/ARM/readtp.ll
+++ llvm/test/CodeGen/ARM/readtp.ll
@@ -1,5 +1,7 @@
 ; RUN: llc -mtriple=armeb-linux-gnueabihf -O2 -mattr=+read-tp-hard %s -o - | FileCheck %s -check-prefix=CHECK-HARD
 ; RUN: llc -mtriple=armeb-linux-gnueabihf -O2 %s -o - | FileCheck %s -check-prefix=CHECK-SOFT
+; RUN: llc -mtriple=thumbv7-linux-gnueabihf -O2 -mattr=+read-tp-hard %s -o - | FileCheck %s -check-prefix=CHECK-HARD
+; RUN: llc -mtriple=thumbv7-linux-gnueabihf -O2 %s -o - | FileCheck %s -check-prefix=CHECK-SOFT
 
 
 ; __thread int counter;
Index: llvm/lib/Target/ARM/ARMInstrThumb2.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb2.td
+++ llvm/lib/Target/ARM/ARMInstrThumb2.td
@@ -4671,6 +4671,9 @@
 }
 
 
+// Reading thread pointer from coprocessor register
+def : T2Pat<(ARMthread_pointer), (t2MRC 15, 0, 13, 0, 3)>,
+  Requires<[IsThumb2, IsReadTPHard]>;

[PATCH] D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed

2021-10-27 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb updated this revision to Diff 382826.
ardb added a comment.

Drop new Clang CodeGen test, and add the Thumb2 check to an existing backend 
test instead.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112600/new/

https://reviews.llvm.org/D112600

Files:
  llvm/lib/Target/ARM/ARMInstrThumb.td
  llvm/lib/Target/ARM/ARMInstrThumb2.td
  llvm/test/CodeGen/ARM/readtp.ll


Index: llvm/test/CodeGen/ARM/readtp.ll
===
--- llvm/test/CodeGen/ARM/readtp.ll
+++ llvm/test/CodeGen/ARM/readtp.ll
@@ -1,5 +1,7 @@
 ; RUN: llc -mtriple=armeb-linux-gnueabihf -O2 -mattr=+read-tp-hard %s -o - | 
FileCheck %s -check-prefix=CHECK-HARD
 ; RUN: llc -mtriple=armeb-linux-gnueabihf -O2 %s -o - | FileCheck %s 
-check-prefix=CHECK-SOFT
+; RUN: llc -mtriple=thumbv7-linux-gnueabihf -O2 -mattr=+read-tp-hard %s -o - | 
FileCheck %s -check-prefix=CHECK-HARD
+; RUN: llc -mtriple=thumbv7-linux-gnueabihf -O2 %s -o - | FileCheck %s 
-check-prefix=CHECK-SOFT
 
 
 ; __thread int counter;
Index: llvm/lib/Target/ARM/ARMInstrThumb2.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb2.td
+++ llvm/lib/Target/ARM/ARMInstrThumb2.td
@@ -4671,6 +4671,9 @@
 }
 
 
+// Reading thread pointer from coprocessor register
+def : T2Pat<(ARMthread_pointer), (t2MRC 15, 0, 13, 0, 3)>,
+  Requires<[IsThumb2, IsReadTPHard]>;
 
 
//===--===//
 // ARMv8.1 Privilege Access Never extension
Index: llvm/lib/Target/ARM/ARMInstrThumb.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb.td
+++ llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -1520,6 +1520,7 @@
 let isCall = 1, Defs = [R0, R12, LR, CPSR], Uses = [SP] in
 def tTPsoft : tPseudoInst<(outs), (ins), 4, IIC_Br,
   [(set R0, ARMthread_pointer)]>,
+  Requires<[IsThumb, IsReadTPSoft]>,
   Sched<[WriteBr]>;
 
 
//===--===//


Index: llvm/test/CodeGen/ARM/readtp.ll
===
--- llvm/test/CodeGen/ARM/readtp.ll
+++ llvm/test/CodeGen/ARM/readtp.ll
@@ -1,5 +1,7 @@
 ; RUN: llc -mtriple=armeb-linux-gnueabihf -O2 -mattr=+read-tp-hard %s -o - | FileCheck %s -check-prefix=CHECK-HARD
 ; RUN: llc -mtriple=armeb-linux-gnueabihf -O2 %s -o - | FileCheck %s -check-prefix=CHECK-SOFT
+; RUN: llc -mtriple=thumbv7-linux-gnueabihf -O2 -mattr=+read-tp-hard %s -o - | FileCheck %s -check-prefix=CHECK-HARD
+; RUN: llc -mtriple=thumbv7-linux-gnueabihf -O2 %s -o - | FileCheck %s -check-prefix=CHECK-SOFT
 
 
 ; __thread int counter;
Index: llvm/lib/Target/ARM/ARMInstrThumb2.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb2.td
+++ llvm/lib/Target/ARM/ARMInstrThumb2.td
@@ -4671,6 +4671,9 @@
 }
 
 
+// Reading thread pointer from coprocessor register
+def : T2Pat<(ARMthread_pointer), (t2MRC 15, 0, 13, 0, 3)>,
+  Requires<[IsThumb2, IsReadTPHard]>;
 
 //===--===//
 // ARMv8.1 Privilege Access Never extension
Index: llvm/lib/Target/ARM/ARMInstrThumb.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb.td
+++ llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -1520,6 +1520,7 @@
 let isCall = 1, Defs = [R0, R12, LR, CPSR], Uses = [SP] in
 def tTPsoft : tPseudoInst<(outs), (ins), 4, IIC_Br,
   [(set R0, ARMthread_pointer)]>,
+  Requires<[IsThumb, IsReadTPSoft]>,
   Sched<[WriteBr]>;
 
 //===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed

2021-10-27 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb added inline comments.



Comment at: clang/test/CodeGen/arm-tphard.c:10
+}
+

nickdesaulniers wrote:
> Let's make this a test under llvm/test/CodeGen/, using IR:
> ```
> ; RUN: llc --mtriple=armv7-linux-gnueabihf -o - %s | FileCheck %s
> ; RUN: llc --mtriple=thumbv7-linux-gnu -o - %s | FileCheck %s
> define dso_local i8* @tphard() "target-features"="+read-tp-hard" {
> // CHECK-NOT: __aeabi_read_tp
>   %1 = tail call i8* @llvm.thread.pointer()
>   ret i8* %1
> }
> 
> declare i8* @llvm.thread.pointer()
> ```
> 
> Let's make this a test under llvm/test/CodeGen/, using IR:

Why is that better?

> ```
> ; RUN: llc --mtriple=armv7-linux-gnueabihf -o - %s | FileCheck %s
> ; RUN: llc --mtriple=thumbv7-linux-gnu -o - %s | FileCheck %s

Are you sure using this triple forces generation of Thumb2 code? It didn't seem 
to when I tried.

> define dso_local i8* @tphard() "target-features"="+read-tp-hard" {
> // CHECK-NOT: __aeabi_read_tp

Are you sure __aeabi_read_tp will appear in the IR for soft TP?

>   %1 = tail call i8* @llvm.thread.pointer()
>   ret i8* %1
> }
> 
> declare i8* @llvm.thread.pointer()
> ```
> 




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112600/new/

https://reviews.llvm.org/D112600

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed

2021-10-27 Thread Ard Biesheuvel via Phabricator via cfe-commits 
ardb created this revision.
ardb added reviewers: nickdesaulniers, nathanchance, psmith.
Herald added subscribers: hiraditya, kristof.beyls.
ardb requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

In ARM mode, passing -mtp=cp15 forces the use of an inline MRC system register 
read to move the thread pointer value into a register.

Currently, in Thumb2 mode, -mtp=cp15 is ignored, and a call to the 
__aeabi_read_tp helper is emitted instead.

This is inconsistent, and breaks the Linux/ARM build for Thumb2 targets, as the 
Linux kernel does not provide an implementation of __aeabi_read_tp,.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112600

Files:
  clang/test/CodeGen/arm-tphard.c
  llvm/lib/Target/ARM/ARMInstrThumb.td
  llvm/lib/Target/ARM/ARMInstrThumb2.td


Index: llvm/lib/Target/ARM/ARMInstrThumb2.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb2.td
+++ llvm/lib/Target/ARM/ARMInstrThumb2.td
@@ -4670,7 +4670,9 @@
   let DecoderNamespace = "Thumb2CoProc";
 }
 
-
+// Reading thread pointer from coprocessor register
+def : T2Pat<(ARMthread_pointer), (t2MRC 15, 0, 13, 0, 3)>,
+  Requires<[IsThumb2, IsReadTPHard]>;
 
 
//===--===//
 // ARMv8.1 Privilege Access Never extension
Index: llvm/lib/Target/ARM/ARMInstrThumb.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb.td
+++ llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -1520,6 +1520,7 @@
 let isCall = 1, Defs = [R0, R12, LR, CPSR], Uses = [SP] in
 def tTPsoft : tPseudoInst<(outs), (ins), 4, IIC_Br,
   [(set R0, ARMthread_pointer)]>,
+  Requires<[IsThumb, IsReadTPSoft]>,
   Sched<[WriteBr]>;
 
 
//===--===//
Index: clang/test/CodeGen/arm-tphard.c
===
--- /dev/null
+++ clang/test/CodeGen/arm-tphard.c
@@ -0,0 +1,10 @@
+// REQUIRES: arm-registered-target
+
+// RUN: %clang -target armv7-linux-gnueabihf -mtp=cp15 -S -o - %s | FileCheck 
%s
+// RUN: %clang -target armv7-linux-gnueabihf -mtp=cp15 -mthumb -S -o - %s | 
FileCheck %s
+
+void *tphard(void) {
+// CHECK-NOT: __aeabi_read_tp
+  return __builtin_thread_pointer();
+}
+


Index: llvm/lib/Target/ARM/ARMInstrThumb2.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb2.td
+++ llvm/lib/Target/ARM/ARMInstrThumb2.td
@@ -4670,7 +4670,9 @@
   let DecoderNamespace = "Thumb2CoProc";
 }
 
-
+// Reading thread pointer from coprocessor register
+def : T2Pat<(ARMthread_pointer), (t2MRC 15, 0, 13, 0, 3)>,
+  Requires<[IsThumb2, IsReadTPHard]>;
 
 //===--===//
 // ARMv8.1 Privilege Access Never extension
Index: llvm/lib/Target/ARM/ARMInstrThumb.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb.td
+++ llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -1520,6 +1520,7 @@
 let isCall = 1, Defs = [R0, R12, LR, CPSR], Uses = [SP] in
 def tTPsoft : tPseudoInst<(outs), (ins), 4, IIC_Br,
   [(set R0, ARMthread_pointer)]>,
+  Requires<[IsThumb, IsReadTPSoft]>,
   Sched<[WriteBr]>;
 
 //===--===//
Index: clang/test/CodeGen/arm-tphard.c
===
--- /dev/null
+++ clang/test/CodeGen/arm-tphard.c
@@ -0,0 +1,10 @@
+// REQUIRES: arm-registered-target
+
+// RUN: %clang -target armv7-linux-gnueabihf -mtp=cp15 -S -o - %s | FileCheck %s
+// RUN: %clang -target armv7-linux-gnueabihf -mtp=cp15 -mthumb -S -o - %s | FileCheck %s
+
+void *tphard(void) {
+// CHECK-NOT: __aeabi_read_tp
+  return __builtin_thread_pointer();
+}
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits