diff mbox

x86_64: fix 'gather' avx2 builtin names

Message ID 20160925134819.25926-1-slyich@gmail.com
State New
Headers show

Commit Message

Sergei Trofimovich Sept. 25, 2016, 1:48 p.m. UTC 
From: Sergei Trofimovich <siarheit@google.com>

Today I traced AVX2 optimisation bug in gcc and
distilled it down to '__builtin_ia32_gatheraltdiv4si256'
generated by gcc.

When I attempted to use this builtin directly
in a simple program gcc refused to recognise
it as known:

  #include <immintrin.h>

  void a (void) {
    __builtin_ia32_gatheraltdiv4si256 (1);
  }

  $ LANG=C gcc -c -march=core-avx2 a.c
    a.c: In function 'a':
    a.c:4:5: warning: implicit declaration of function
        '__builtin_ia32_gatheraltdiv4si256' [-Wimplicit-function-declaration]

The cause is trailing whitespace in builtin definition:

      def_builtin (OPTION_MASK_ISA_AVX2, "__builtin_ia32_gatheraltdiv4si256 ",

Fixed by dropping trailing whitespace.

Signed-off-by: Sergei Trofimovich <siarheit@google.com>
---
 gcc/config/i386/i386.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

Comments

Jakub Jelinek Sept. 25, 2016, 5:01 p.m. UTC | #1 
On Sun, Sep 25, 2016 at 02:48:19PM +0100, Sergei Trofimovich wrote:
> From: Sergei Trofimovich <siarheit@google.com>
> 
> Today I traced AVX2 optimisation bug in gcc and
> distilled it down to '__builtin_ia32_gatheraltdiv4si256'
> generated by gcc.
> 
> When I attempted to use this builtin directly
> in a simple program gcc refused to recognise
> it as known:
> 
>   #include <immintrin.h>
> 
>   void a (void) {
>     __builtin_ia32_gatheraltdiv4si256 (1);
>   }
> 
>   $ LANG=C gcc -c -march=core-avx2 a.c
>     a.c: In function 'a':
>     a.c:4:5: warning: implicit declaration of function
>         '__builtin_ia32_gatheraltdiv4si256' [-Wimplicit-function-declaration]
> 
> The cause is trailing whitespace in builtin definition:
> 
>       def_builtin (OPTION_MASK_ISA_AVX2, "__builtin_ia32_gatheraltdiv4si256 ",
> 
> Fixed by dropping trailing whitespace.

No, that isn't a fix, adding such characters has been an older way to
express internal only builtins that aren't ever meant to be available to
users (these days we'd use internal-fn.* stuff, except we don't have support
for backends adding their own internal functions).

	Jakub
Sergei Trofimovich Sept. 25, 2016, 6:08 p.m. UTC | #2 
On Sun, 25 Sep 2016 19:01:14 +0200
Jakub Jelinek <jakub@redhat.com> wrote:

> On Sun, Sep 25, 2016 at 02:48:19PM +0100, Sergei Trofimovich wrote:
> > From: Sergei Trofimovich <siarheit@google.com>
> > 
> > Today I traced AVX2 optimisation bug in gcc and
> > distilled it down to '__builtin_ia32_gatheraltdiv4si256'
> > generated by gcc.
> > 
> > When I attempted to use this builtin directly
> > in a simple program gcc refused to recognise
> > it as known:
> > 
> >   #include <immintrin.h>
> > 
> >   void a (void) {
> >     __builtin_ia32_gatheraltdiv4si256 (1);
> >   }
> > 
> >   $ LANG=C gcc -c -march=core-avx2 a.c
> >     a.c: In function 'a':
> >     a.c:4:5: warning: implicit declaration of function
> >         '__builtin_ia32_gatheraltdiv4si256' [-Wimplicit-function-declaration]
> > 
> > The cause is trailing whitespace in builtin definition:
> > 
> >       def_builtin (OPTION_MASK_ISA_AVX2, "__builtin_ia32_gatheraltdiv4si256 ",
> > 
> > Fixed by dropping trailing whitespace.  
> 
> No, that isn't a fix, adding such characters has been an older way to
> express internal only builtins that aren't ever meant to be available to
> users (these days we'd use internal-fn.* stuff, except we don't have support
> for backends adding their own internal functions).

Aha, got it. Having a seemingly callable name is very confusing.

I did grab the name from one of -fdump-tree-all outputs
(bug.c.165t.optimized) which looked as:

  vect_.16_59 = VIEW_CONVERT_EXPR<vector(4) long long int>(vect_var_.12_56);
  vect_var_.14_60 = __builtin_ia32_gatheraltdiv4si256  ({ -1, -1, -1, -1, -1, -1, -1, -1 }, &PERMS, vect_.16_59, { -1, -1, -1, 
-1, -1, -1, -1, -1 }, 4);
  vect_.17_61 = VIEW_CONVERT_EXPR<vector(4) long long int>(vect_var_.12_57);
  vect_var_.14_62 = __builtin_ia32_gatheraltdiv4si256  ({ -1, -1, -1, -1, -1, -1, -1, -1 }, &PERMS, vect_.17_61, { -1, -1, -1, 
-1, -1, -1, -1, -1 }, 4);

and I assumed that builtin can be used directly to reproduce
smaller example. Sorry for the noise.
diff mbox

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 143b905..48a5354 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -31403,19 +31403,19 @@  ix86_init_mmx_sse_builtins (void)
 	       V4SI_FTYPE_V4SI_PCINT_V4DI_V4SI_INT,
 	       IX86_BUILTIN_GATHERDIV8SI);
 
-  def_builtin (OPTION_MASK_ISA_AVX2, "__builtin_ia32_gatheraltsiv4df ",
+  def_builtin (OPTION_MASK_ISA_AVX2, "__builtin_ia32_gatheraltsiv4df",
 	       V4DF_FTYPE_V4DF_PCDOUBLE_V8SI_V4DF_INT,
 	       IX86_BUILTIN_GATHERALTSIV4DF);
 
-  def_builtin (OPTION_MASK_ISA_AVX2, "__builtin_ia32_gatheraltdiv4sf256 ",
+  def_builtin (OPTION_MASK_ISA_AVX2, "__builtin_ia32_gatheraltdiv4sf256",
 	       V8SF_FTYPE_V8SF_PCFLOAT_V4DI_V8SF_INT,
 	       IX86_BUILTIN_GATHERALTDIV8SF);
 
-  def_builtin (OPTION_MASK_ISA_AVX2, "__builtin_ia32_gatheraltsiv4di ",
+  def_builtin (OPTION_MASK_ISA_AVX2, "__builtin_ia32_gatheraltsiv4di",
 	       V4DI_FTYPE_V4DI_PCINT64_V8SI_V4DI_INT,
 	       IX86_BUILTIN_GATHERALTSIV4DI);
 
-  def_builtin (OPTION_MASK_ISA_AVX2, "__builtin_ia32_gatheraltdiv4si256 ",
+  def_builtin (OPTION_MASK_ISA_AVX2, "__builtin_ia32_gatheraltdiv4si256",
 	       V8SI_FTYPE_V8SI_PCINT_V4DI_V8SI_INT,
 	       IX86_BUILTIN_GATHERALTDIV8SI);
 
@@ -31452,19 +31452,19 @@  ix86_init_mmx_sse_builtins (void)
 	       V8DI_FTYPE_V8DI_PCINT64_V8DI_QI_INT,
 	       IX86_BUILTIN_GATHER3DIV8DI);
 
-  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_gatheraltsiv8df ",
+  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_gatheraltsiv8df",
 	       V8DF_FTYPE_V8DF_PCDOUBLE_V16SI_QI_INT,
 	       IX86_BUILTIN_GATHER3ALTSIV8DF);
 
-  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_gatheraltdiv8sf ",
+  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_gatheraltdiv8sf",
 	       V16SF_FTYPE_V16SF_PCFLOAT_V8DI_HI_INT,
 	       IX86_BUILTIN_GATHER3ALTDIV16SF);
 
-  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_gatheraltsiv8di ",
+  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_gatheraltsiv8di",
 	       V8DI_FTYPE_V8DI_PCINT64_V16SI_QI_INT,
 	       IX86_BUILTIN_GATHER3ALTSIV8DI);
 
-  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_gatheraltdiv8si ",
+  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_gatheraltdiv8si",
 	       V16SI_FTYPE_V16SI_PCINT_V8DI_HI_INT,
 	       IX86_BUILTIN_GATHER3ALTDIV16SI);
 
@@ -31565,19 +31565,19 @@  ix86_init_mmx_sse_builtins (void)
 	       V4SI_FTYPE_V4SI_PCINT_V4DI_QI_INT,
 	       IX86_BUILTIN_GATHER3DIV8SI);
 
-  def_builtin (OPTION_MASK_ISA_AVX512VL, "__builtin_ia32_gather3altsiv4df ",
+  def_builtin (OPTION_MASK_ISA_AVX512VL, "__builtin_ia32_gather3altsiv4df",
 	       V4DF_FTYPE_V4DF_PCDOUBLE_V8SI_QI_INT,
 	       IX86_BUILTIN_GATHER3ALTSIV4DF);
 
-  def_builtin (OPTION_MASK_ISA_AVX512VL, "__builtin_ia32_gather3altdiv8sf ",
+  def_builtin (OPTION_MASK_ISA_AVX512VL, "__builtin_ia32_gather3altdiv8sf",
 	       V8SF_FTYPE_V8SF_PCFLOAT_V4DI_QI_INT,
 	       IX86_BUILTIN_GATHER3ALTDIV8SF);
 
-  def_builtin (OPTION_MASK_ISA_AVX512VL, "__builtin_ia32_gather3altsiv4di ",
+  def_builtin (OPTION_MASK_ISA_AVX512VL, "__builtin_ia32_gather3altsiv4di",
 	       V4DI_FTYPE_V4DI_PCINT64_V8SI_QI_INT,
 	       IX86_BUILTIN_GATHER3ALTSIV4DI);
 
-  def_builtin (OPTION_MASK_ISA_AVX512VL, "__builtin_ia32_gather3altdiv8si ",
+  def_builtin (OPTION_MASK_ISA_AVX512VL, "__builtin_ia32_gather3altdiv8si",
 	       V8SI_FTYPE_V8SI_PCINT_V4DI_QI_INT,
 	       IX86_BUILTIN_GATHER3ALTDIV8SI);
 
@@ -31644,19 +31644,19 @@  ix86_init_mmx_sse_builtins (void)
   def_builtin (OPTION_MASK_ISA_AVX512VL, "__builtin_ia32_scatterdiv2di",
 	       VOID_FTYPE_PLONGLONG_QI_V2DI_V2DI_INT,
 	       IX86_BUILTIN_SCATTERDIV2DI);
-  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_scatteraltsiv8df ",
+  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_scatteraltsiv8df",
 	       VOID_FTYPE_PDOUBLE_QI_V16SI_V8DF_INT,
 	       IX86_BUILTIN_SCATTERALTSIV8DF);
 
-  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_scatteraltdiv8sf ",
+  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_scatteraltdiv8sf",
 	       VOID_FTYPE_PFLOAT_HI_V8DI_V16SF_INT,
 	       IX86_BUILTIN_SCATTERALTDIV16SF);
 
-  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_scatteraltsiv8di ",
+  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_scatteraltsiv8di",
 	       VOID_FTYPE_PLONGLONG_QI_V16SI_V8DI_INT,
 	       IX86_BUILTIN_SCATTERALTSIV8DI);
 
-  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_scatteraltdiv8si ",
+  def_builtin (OPTION_MASK_ISA_AVX512F, "__builtin_ia32_scatteraltdiv8si",
 	       VOID_FTYPE_PINT_HI_V8DI_V16SI_INT,
 	       IX86_BUILTIN_SCATTERALTDIV16SI);