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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
