| Message ID | CAMZc-byJ5XWCRdpm_1OxBD_JRNhdreJE21xAdbcg_xj7eQC9bQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Implement __builtin_thread_pointer for x86 TLS | expand |
On Tue, Sep 08, 2020 at 04:14:52PM +0800, Hongtao Liu wrote: > Hi: > We have "*load_tp_<mode>" in i386.md for load of thread pointer in > i386.md, so this patch merely adds the expander for > __builtin_thread_pointer. > > Bootstrap is ok, regression test is ok for i386/x86-64 backend. > Ok for trunk? > > gcc/ChangeLog: > PR target/96955 > * config/i386/i386.md (get_thread_pointer<mode>): New > expander. I wonder if this shouldn't be done only if targetm.have_tls is true. Because on targets that use emulated TLS it doesn't really make much sense. > gcc/testsuite/ChangeLog: > > * gcc.target/i386/pr96955-builtin_thread_pointer.c: New test. The testcase naming is weird. Either call it pr96955.c, or builtin_thread_pointer.c, but not both. > From 4d80571d325859f75b6eb896a0def9695fb65c49 Mon Sep 17 00:00:00 2001 > From: liuhongt <hongtao.liu@intel.com> > Date: Tue, 8 Sep 2020 15:44:58 +0800 > Subject: [PATCH] Implement __builtin_thread_pointer for x86 TLS. > > gcc/ChangeLog: > PR target/96955 > * config/i386/i386.md (get_thread_pointer<mode>): New > expander. > > gcc/testsuite/ChangeLog: > > * gcc.target/i386/pr96955-builtin_thread_pointer.c: New test. > --- > gcc/config/i386/i386.md | 5 ++++ > .../i386/pr96955-builtin_thread_pointer.c | 28 +++++++++++++++++++ > 2 files changed, 33 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/i386/pr96955-builtin_thread_pointer.c > > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > index 446793b78db..55b1852cf9a 100644 > --- a/gcc/config/i386/i386.md > +++ b/gcc/config/i386/i386.md > @@ -15433,6 +15433,11 @@ (define_insn_and_split "*tls_local_dynamic_32_once" > (clobber (reg:CC FLAGS_REG))])]) > > ;; Load and add the thread base pointer from %<tp_seg>:0. > +(define_expand "get_thread_pointer<mode>" > + [(set (match_operand:PTR 0 "register_operand") > + (unspec:PTR [(const_int 0)] UNSPEC_TP))] > + "") > + > (define_insn_and_split "*load_tp_<mode>" > [(set (match_operand:PTR 0 "register_operand" "=r") > (unspec:PTR [(const_int 0)] UNSPEC_TP))] > diff --git a/gcc/testsuite/gcc.target/i386/pr96955-builtin_thread_pointer.c b/gcc/testsuite/gcc.target/i386/pr96955-builtin_thread_pointer.c > new file mode 100644 > index 00000000000..dce31488117 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr96955-builtin_thread_pointer.c > @@ -0,0 +1,28 @@ > +/* { dg-do compile } */ > +/* { dg-options "-mtls-direct-seg-refs -O2 -masm=att" } */ > + > +int* > +foo1 () > +{ > + return (int*) __builtin_thread_pointer (); > +} > + > +/* { dg-final { scan-assembler "mov\[lq\]\[ \t\]*%\[fg\]s:0, %\[re\]ax" } } */ > + > +int > +foo2 () > +{ > + int* p = (int*) __builtin_thread_pointer (); > + return p[4]; > +} > + > +/* { dg-final { scan-assembler "movl\[ \t\]*%\[fg\]s:16, %eax" } } */ > + > +int > +foo3 (int i) > +{ > + int* p = (int*) __builtin_thread_pointer (); > + return p[i]; > +} > + > +/* { dg-final { scan-assembler "movl\[ \t\]*%\[fg\]s:0\\(,%\[a-z0-9\]*,4\\), %eax" } } */ > -- > 2.18.1 > Jakub
On Tue, Sep 8, 2020 at 4:52 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Sep 08, 2020 at 04:14:52PM +0800, Hongtao Liu wrote: > > Hi: > > We have "*load_tp_<mode>" in i386.md for load of thread pointer in > > i386.md, so this patch merely adds the expander for > > __builtin_thread_pointer. > > > > Bootstrap is ok, regression test is ok for i386/x86-64 backend. > > Ok for trunk? > > > > gcc/ChangeLog: > > PR target/96955 > > * config/i386/i386.md (get_thread_pointer<mode>): New > > expander. > > I wonder if this shouldn't be done only if targetm.have_tls is true. > Because on targets that use emulated TLS it doesn't really make much sense. > Changed as ;; Load and add the thread base pointer from %<tp_seg>:0. +(define_expand "get_thread_pointer<mode>" + [(set (match_operand:PTR 0 "register_operand") + (unspec:PTR [(const_int 0)] UNSPEC_TP))] + "" +{ + /* targetm is not existed in the scope of condition. */ + if (!targetm.have_tls) + error ("%<__builtin_thread_pointer%> is not supported on this target"); +}) > > gcc/testsuite/ChangeLog: > > > > * gcc.target/i386/pr96955-builtin_thread_pointer.c: New test. > > The testcase naming is weird. Either call it pr96955.c, or > builtin_thread_pointer.c, but not both. > Renamed to builtin_thread_pointer.c. Update patch.
On Wed, Sep 09, 2020 at 10:30:46AM +0800, Hongtao Liu wrote: > From 400418fadce46e7db7bd37be45ef5ff5beb08d19 Mon Sep 17 00:00:00 2001 > From: liuhongt <hongtao.liu@intel.com> > Date: Tue, 8 Sep 2020 15:44:58 +0800 > Subject: [PATCH] Implement __builtin_thread_pointer for x86 TLS. > > gcc/ChangeLog: > PR target/96955 > * config/i386/i386.md (get_thread_pointer<mode>): New > expander. > > gcc/testsuite/ChangeLog: > > * gcc.target/i386/builtin_thread_pointer.c: New test. > --- > gcc/config/i386/i386.md | 10 +++++++ > .../gcc.target/i386/builtin_thread_pointer.c | 28 +++++++++++++++++++ > 2 files changed, 38 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/i386/builtin_thread_pointer.c > > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > index 446793b78db..2f6eb0a7b98 100644 > --- a/gcc/config/i386/i386.md > +++ b/gcc/config/i386/i386.md > @@ -15433,6 +15433,16 @@ (define_insn_and_split "*tls_local_dynamic_32_once" > (clobber (reg:CC FLAGS_REG))])]) > > ;; Load and add the thread base pointer from %<tp_seg>:0. > +(define_expand "get_thread_pointer<mode>" > + [(set (match_operand:PTR 0 "register_operand") > + (unspec:PTR [(const_int 0)] UNSPEC_TP))] > + "" > +{ > + /* targetm is not existed in the scope of condition. */ Reword as "targetm is not visible in the scope of the condition." In fact, even if it was, it wouldn't help, because expand_builtin_thread_pointer assumes that if the expander exists, then it will work and emit some code and emits the error only if the expander doesn't exist. Ok for trunk with that change, thanks. > + if (!targetm.have_tls) > + error ("%<__builtin_thread_pointer%> is not supported on this target"); > +}) > + Jakub
On Wed, Sep 9, 2020 at 2:35 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Wed, Sep 09, 2020 at 10:30:46AM +0800, Hongtao Liu wrote: > > From 400418fadce46e7db7bd37be45ef5ff5beb08d19 Mon Sep 17 00:00:00 2001 > > From: liuhongt <hongtao.liu@intel.com> > > Date: Tue, 8 Sep 2020 15:44:58 +0800 > > Subject: [PATCH] Implement __builtin_thread_pointer for x86 TLS. > > > > gcc/ChangeLog: > > PR target/96955 > > * config/i386/i386.md (get_thread_pointer<mode>): New > > expander. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/i386/builtin_thread_pointer.c: New test. > > --- > > gcc/config/i386/i386.md | 10 +++++++ > > .../gcc.target/i386/builtin_thread_pointer.c | 28 +++++++++++++++++++ > > 2 files changed, 38 insertions(+) > > create mode 100644 gcc/testsuite/gcc.target/i386/builtin_thread_pointer.c > > > > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > > index 446793b78db..2f6eb0a7b98 100644 > > --- a/gcc/config/i386/i386.md > > +++ b/gcc/config/i386/i386.md > > @@ -15433,6 +15433,16 @@ (define_insn_and_split "*tls_local_dynamic_32_once" > > (clobber (reg:CC FLAGS_REG))])]) > > > > ;; Load and add the thread base pointer from %<tp_seg>:0. > > +(define_expand "get_thread_pointer<mode>" > > + [(set (match_operand:PTR 0 "register_operand") > > + (unspec:PTR [(const_int 0)] UNSPEC_TP))] > > + "" > > +{ > > + /* targetm is not existed in the scope of condition. */ > > Reword as "targetm is not visible in the scope of the condition." > In fact, even if it was, it wouldn't help, because expand_builtin_thread_pointer > assumes that if the expander exists, then it will work and emit some code > and emits the error only if the expander doesn't exist. > > Ok for trunk with that change, thanks. > Thanks for the review. > > + if (!targetm.have_tls) > > + error ("%<__builtin_thread_pointer%> is not supported on this target"); > > +}) > > + > > Jakub >
From 4d80571d325859f75b6eb896a0def9695fb65c49 Mon Sep 17 00:00:00 2001 From: liuhongt <hongtao.liu@intel.com> Date: Tue, 8 Sep 2020 15:44:58 +0800 Subject: [PATCH] Implement __builtin_thread_pointer for x86 TLS. gcc/ChangeLog: PR target/96955 * config/i386/i386.md (get_thread_pointer<mode>): New expander. gcc/testsuite/ChangeLog: * gcc.target/i386/pr96955-builtin_thread_pointer.c: New test. --- gcc/config/i386/i386.md | 5 ++++ .../i386/pr96955-builtin_thread_pointer.c | 28 +++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 gcc/testsuite/gcc.target/i386/pr96955-builtin_thread_pointer.c diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 446793b78db..55b1852cf9a 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -15433,6 +15433,11 @@ (define_insn_and_split "*tls_local_dynamic_32_once" (clobber (reg:CC FLAGS_REG))])]) ;; Load and add the thread base pointer from %<tp_seg>:0. +(define_expand "get_thread_pointer<mode>" + [(set (match_operand:PTR 0 "register_operand") + (unspec:PTR [(const_int 0)] UNSPEC_TP))] + "") + (define_insn_and_split "*load_tp_<mode>" [(set (match_operand:PTR 0 "register_operand" "=r") (unspec:PTR [(const_int 0)] UNSPEC_TP))] diff --git a/gcc/testsuite/gcc.target/i386/pr96955-builtin_thread_pointer.c b/gcc/testsuite/gcc.target/i386/pr96955-builtin_thread_pointer.c new file mode 100644 index 00000000000..dce31488117 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr96955-builtin_thread_pointer.c @@ -0,0 +1,28 @@ +/* { dg-do compile } */ +/* { dg-options "-mtls-direct-seg-refs -O2 -masm=att" } */ + +int* +foo1 () +{ + return (int*) __builtin_thread_pointer (); +} + +/* { dg-final { scan-assembler "mov\[lq\]\[ \t\]*%\[fg\]s:0, %\[re\]ax" } } */ + +int +foo2 () +{ + int* p = (int*) __builtin_thread_pointer (); + return p[4]; +} + +/* { dg-final { scan-assembler "movl\[ \t\]*%\[fg\]s:16, %eax" } } */ + +int +foo3 (int i) +{ + int* p = (int*) __builtin_thread_pointer (); + return p[i]; +} + +/* { dg-final { scan-assembler "movl\[ \t\]*%\[fg\]s:0\\(,%\[a-z0-9\]*,4\\), %eax" } } */ -- 2.18.1
Hi: We have "*load_tp_<mode>" in i386.md for load of thread pointer in i386.md, so this patch merely adds the expander for __builtin_thread_pointer. Bootstrap is ok, regression test is ok for i386/x86-64 backend. Ok for trunk? gcc/ChangeLog: PR target/96955 * config/i386/i386.md (get_thread_pointer<mode>): New expander. gcc/testsuite/ChangeLog: * gcc.target/i386/pr96955-builtin_thread_pointer.c: New test.