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

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.


================
Comment at: clang/test/CodeGen/arm-tphard.c:10
+}
+
----------------
ardb wrote:
> 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()
> > ```
> > 
> 
> 
> Why is that better?

Because the front-end isn't really involved in this back end code gen bug, so we should just be testing the back end.

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

Good question; I guess for thumb2 there's no command line flag passed to the compiler that says "I would like thumb[1] as opposed to thumb2?"  Maybe @peter.smith can provide some color on that? Is it simply armv7 for thumb2 vs v6 for thumb[1], or something else?

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

`__aeabi_read_tp` will never appear in the IR (unless someone explicitly called it`. Instead, we're testing that the intrinsic (`@llvm.thread.pointer`) is lowered to either that libcall or `mrc`.  `__aeabi_read_tp` may appear in the object file or assembler code generated from that intrinsic call.

But also, it looks like there's already coverage for `__aeabi_read_tp` being generated for soft TP. See also llvm/test/CodeGen/ARM/readtp.ll. There's also thread_pointer.ll in that same dir (and a few more tests mentioning `__aeabi_read_tp` that all look like candidates to add these tests to rather than creating a new test file, perhaps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112600