[Bug target/102166] New: [i386] AMX intrinsics and macros not defined in C++

[Bug target/102166] New: [i386] AMX intrinsics and macros not defined in C++

[thiago at kde dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

            Bug ID: 102166
           Summary: [i386] AMX intrinsics and macros not defined in C++
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: thiago at kde dot org
  Target Milestone: ---

$ cat test.cpp
#include <immintrin.h>

__attribute__((target("avx"))) void avx()
{
    _mm256_zeroall();
}

__attribute__((target("amx-tile"))) void amx()
{
    _tile_loadd(0, 0, 0);
    _tile_release();
}
$ gcc -c test.cpp
test.cpp: In function ‘void amx()’:
test.cpp:10:5: error: ‘_tile_loadd’ was not declared in this scope
   10 |     _tile_loadd(0, 0, 0);
      |     ^~~~~~~~~~~
test.cpp:11:5: error: ‘_tile_release’ was not declared in this scope
   11 |     _tile_release();
      |     ^~~~~~~~~~~~~

That's because the macros and intrinsics in amxtileintrin.h are only defined
behind:

#if defined(__x86_64__) && defined(__AMX_TILE__)

The __AMX_TILE__ macro isn't defined and doesn't need to be. None of the other
itnrinsics require compiling with -m options. In fact, code shouldn't use -m
options for things that are detected at runtime, like AMX inevitably has to be.

[Bug target/102166] [i386] AMX intrinsics and macros not defined in C++

[thiago at kde dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

Thiago Macieira <thiago at kde dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl.tools at gmail dot com

--- Comment #1 from Thiago Macieira <thiago at kde dot org> ---
I don't understand how this compiles in C mode:

$ gcc -O2 -Werror=implicit-function-declaration -c -xc test.cpp
$ gcc -O2 -fno-asynchronous-unwind-tables  -S -o - -xc test.cpp    
        .file   "test.cpp"
        .text
        .p2align 4
        .globl  avx
        .type   avx, @function
avx:
        vzeroall
        ret
        .size   avx, .-avx
        .p2align 4
        .globl  amx
        .type   amx, @function
amx:
        xorl    %eax, %eax
#APP
# 10 "test.cpp" 1
        tileloadd       (%rax,%rax,1), %tmm0
# 0 "" 2
# 56 "/usr/lib64/gcc/x86_64-generic-linux/11/include/amxtileintrin.h" 1
        tilerelease
# 0 "" 2
#NO_APP
        ret
        .size   amx, .-amx

The comments in the assembly output indicate that _tile_loadd is a macro that
was expanded from test.cpp and that _tile_release is a function in
amxtileintrin.h.

But neither is defined.

$ gcc -E -xc test.cpp | grep -e _tile_loadd -e _tile_release        
    _tile_loadd(0, 0, 0);
    _tile_release();

Whatever they are, they are not visible to the preprocessor.

[Bug target/102166] [i386] AMX intrinsics and macros not defined in C++

[thiago at kde dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

--- Comment #2 from Thiago Macieira <thiago at kde dot org> ---
FYI:

$ cat test.cpp
#include <immintrin.h>

__attribute__((target("avx"))) void avx()
{
    _mm256_zeroall();
}

#ifndef __INTEL_COMPILER
__attribute__((target("amx-tile")))
#endif
void amx()
{
    _tile_loadd(0, 0, 0);
    _tile_release();
}
$ icc -c test.cpp && echo success
success
$ icc -c -xc test.cpp && echo success
success
$ clang -c -xc test.cpp && echo success
success
$ clang -c test.cpp && echo success    
success

$ clang --version
clang version 11.1.0
Target: x86_64-generic-linux
Thread model: posix
InstalledDir: /usr/bin
$ icc --version
icc (ICC) 19.1.3.304 20200925
Copyright (C) 1985-2020 Intel Corporation.  All rights reserved.

[Bug target/102166] [i386] AMX intrinsics and macros not defined in C++

[thiago at kde dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

--- Comment #3 from Thiago Macieira <thiago at kde dot org> ---
There appears to be some preprocessor magic behind the scenes because the
preprocessed output can't be compiled either:

$ gcc -no-integrated-cpp -Werror=implicit-function-declaration -c -xc test.cpp
test.cpp: In function ‘amx’:
test.cpp:10:5: error: implicit declaration of function ‘_tile_loadd’
[-Werror=implicit-function-declaration]
   10 |     _tile_loadd(0, 0, 0);
      |     ^~~~~~~~~~~
test.cpp:11:5: error: implicit declaration of function ‘_tile_release’
[-Werror=implicit-function-declaration]
   11 |     _tile_release();
      |     ^~~~~~~~~~~~~
cc1: some warnings being treated as errors

[Bug target/102166] [i386] AMX intrinsics and macros not defined in C++

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

--- Comment #4 from Hongtao.liu <crazylht at gmail dot com> ---
Because _tile_loadd is implemented as embedded assembly plus macros, if
__AMX_TILE__ is removed, no error will be reported if the user does not use the
-mamx option, So this macro is added here, but obviously this is not convenient
for target_attribute. I think we'd better remove __AMX_TILE__, (not sure why c
doesn't report the error).

[Bug target/102166] [i386] AMX intrinsics and macros not defined in C++

[thiago at kde dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

--- Comment #5 from Thiago Macieira <thiago at kde dot org> ---
(In reply to Hongtao.liu from comment #4)
> Because _tile_loadd is implemented as embedded assembly plus macros, if
> __AMX_TILE__ is removed, no error will be reported if the user does not use
> the -mamx option, So this macro is added here, but obviously this is not
> convenient for target_attribute. I think we'd better remove __AMX_TILE__,
> (not sure why c doesn't report the error).

I suggest doing as Clang did and make it an intrinsic.

[Bug target/102166] [i386] AMX intrinsics and macros not defined in C++

[thiago at kde dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

--- Comment #6 from Thiago Macieira <thiago at kde dot org> ---
> I suggest doing as Clang did and make it an intrinsic.

Or even a __builtin_ia32_markamxtile(); intrinsic, which produces the error if
misused and does add the necessary bits to the .note.gnu.property section

[Bug target/102166] [i386] AMX intrinsics and macros not defined in C++

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

--- Comment #7 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Thiago Macieira from comment #5)
> (In reply to Hongtao.liu from comment #4)
> > Because _tile_loadd is implemented as embedded assembly plus macros, if
> > __AMX_TILE__ is removed, no error will be reported if the user does not use
> > the -mamx option, So this macro is added here, but obviously this is not
> > convenient for target_attribute. I think we'd better remove __AMX_TILE__,
> > (not sure why c doesn't report the error).
> 
> I suggest doing as Clang did and make it an intrinsic.

clang defines them as intrinsic because they support AMX register allocation (a
lot of effort), gcc does not support AMX register allocation for now, and
defining them as intrinsic + builtin doesn't seem to do much good except
provide some error messages.

[Bug target/102166] [i386] AMX intrinsics and macros not defined in C++

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

--- Comment #8 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Thiago Macieira from comment #6)
> > I suggest doing as Clang did and make it an intrinsic.
> 
> Or even a __builtin_ia32_markamxtile(); intrinsic, which produces the error
> if misused and does add the necessary bits to the .note.gnu.property section

_tile_loadconfig/_tile_storeconfig/_tile_release are defined as intrinsics
should be essential for amx program and can provide some error info.

[Bug target/102166] [i386] AMX intrinsics and macros not defined in C++

[thiago at kde dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

--- Comment #9 from Thiago Macieira <thiago at kde dot org> ---
> clang defines them as intrinsic because they support AMX register allocation
> (a lot of effort), gcc does not support AMX register allocation for now, and
> defining them as intrinsic + builtin doesn't seem to do much good except
> provide some error messages.

If you can implement them as macros, I don't see why you need register
allocation in the first place. Just emit the same assembly that is being
emitted now by the inline assembly.

Anyway, I suggest at a minimum removing the #define check. There's little harm
in having no diagnostic on misuse: misuses are probably going to be seen when
testing. Until GCC is able to generate AMX code on its own, the missing
__attribute__ is superfluous anyway.

[Bug target/102166] [i386] AMX intrinsics and macros not defined in C++

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

--- Comment #10 from Hongtao.liu <crazylht at gmail dot com> ---
> 
> Anyway, I suggest at a minimum removing the #define check. There's little
> harm in having no diagnostic on misuse: misuses are probably going to be
> seen when testing. Until GCC is able to generate AMX code on its own, the
> missing __attribute__ is superfluous anyway.

Agree, i'll post a patch for that.

[Bug target/102166] [i386] AMX intrinsics and macros not defined in C++

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by hongtao Liu <liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:de6795bbf58c7085933a1f86a88d8193ea72e26b

commit r12-3323-gde6795bbf58c7085933a1f86a88d8193ea72e26b
Author: liuhongt <hongtao.liu@intel.com>
Date:   Thu Sep 2 12:49:46 2021 +0800

    Remove macro check for __AMX_BF16/INT8/TILE__ in header file.

    gcc/ChangeLog:

            PR target/102166
            * config/i386/amxbf16intrin.h : Remove macro check for
__AMX_BF16__.
            * config/i386/amxint8intrin.h : Remove macro check for
__AMX_INT8__.
            * config/i386/amxtileintrin.h : Remove macro check for
__AMX_TILE__.

    gcc/testsuite/ChangeLog:

            PR target/102166
            * g++.target/i386/pr102166.C: New test.

[Bug target/102166] [i386] AMX intrinsics and macros not defined in C++

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102166

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by hongtao Liu
<liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:259945a0a4880b66f93f71eebe70f78c91e02d5e

commit r11-8953-g259945a0a4880b66f93f71eebe70f78c91e02d5e
Author: liuhongt <hongtao.liu@intel.com>
Date:   Thu Sep 2 12:49:46 2021 +0800

    Remove macro check for __AMX_BF16/INT8/TILE__ in header file.

    gcc/ChangeLog:

            PR target/102166
            * config/i386/amxbf16intrin.h : Remove macro check for
__AMX_BF16__.
            * config/i386/amxint8intrin.h : Remove macro check for
__AMX_INT8__.
            * config/i386/amxtileintrin.h : Remove macro check for
__AMX_TILE__.

    gcc/testsuite/ChangeLog:

            PR target/102166
            * g++.target/i386/pr102166.C: New test.