[committed] amdgcn: disable TImode

[committed] amdgcn: disable TImode

[Andrew Stubbs]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

TImode has always been a problem on amdgcn, and now it is causing many 
new test failures, so I'm disabling it.

The mode only has move instructions defined, which was enough for SLP, 
but any other code trying to use it without checking the optabs is a 
problem.

The mode remains available for use within the backend, which is 
important because at least one hardware instruction uses a TImode value 
with two DImode values packed inside.

Andrew

[-- Attachment #2: 210507-disable-timode.patch --]
[-- Type: text/plain, Size: 842 bytes --]

amdgcn: disable TImode

The TImode support works for moves only, which has worked in most case up
to now, but no longer.

We still need TImode to exist for the instructions that take two DImode
values packed together, but we don't need to advertise this to the middle-end.

gcc/ChangeLog:

	* config/gcn/gcn.c (gcn_scalar_mode_supported_p): Disable TImode.

diff --git a/gcc/config/gcn/gcn.c b/gcc/config/gcn/gcn.c
index 9660ca6eaa4..2baf91d2f1f 100644
--- a/gcc/config/gcn/gcn.c
+++ b/gcc/config/gcn/gcn.c
@@ -361,7 +361,7 @@ gcn_scalar_mode_supported_p (scalar_mode mode)
 	  || mode == HImode /* || mode == HFmode  */
 	  || mode == SImode || mode == SFmode
 	  || mode == DImode || mode == DFmode
-	  || mode == TImode);
+	  /*|| mode == TImode*/); /* TI is used for back-end purposes only.  */
 }
 
 /* Implement TARGET_CLASS_MAX_NREGS.

Re: [committed] amdgcn: disable TImode

[Tobias Burnus]

On 07.05.21 18:35, Andrew Stubbs wrote:

> TImode has always been a problem on amdgcn, and now it is causing many
> new test failures, so I'm disabling it.

Does still still work with libgomp?

The patch sounds as if it might cause problems, but on the other hand,
I assume you did test it? To recall:

The problem is that OpenMP's depobj as implemented in GCC has
sizeof() = 2*sizeof(void*) and is implemented as a two-element struct in C/C++.
But the OpenMP spec mandates that it is an integer type in Fortran, i.e.
integer(kind=omp_depend_kind).

Combining the impl choice and the type requirements that means that
on 64bit systems, this requires __int128 support, cf. commit
https://gcc.gnu.org/g:8d0b2b33748014ee57973c1d7bc9fd7706bb3da9
and https://gcc.gnu.org/PR96306

(Side note: The definition in OpenMP is bad - it should have been
some opaque derived type but that's a mistake done in OpenMP 5.0.)

Tobias

> The mode only has move instructions defined, which was enough for SLP,
> but any other code trying to use it without checking the optabs is a
> problem.
>
> The mode remains available for use within the backend, which is
> important because at least one hardware instruction uses a TImode
> value with two DImode values packed inside.
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

Re: [committed] amdgcn: disable TImode

[Andrew Stubbs]

On 07/05/2021 18:11, Tobias Burnus wrote:
> On 07.05.21 18:35, Andrew Stubbs wrote:
> 
>> TImode has always been a problem on amdgcn, and now it is causing many
>> new test failures, so I'm disabling it.
> 
> Does still still work with libgomp?
> 
> The patch sounds as if it might cause problems, but on the other hand,
> I assume you did test it? To recall:
 >
> The problem is that OpenMP's depobj as implemented in GCC has
> sizeof() = 2*sizeof(void*) and is implemented as a two-element struct in 
> C/C++.
> But the OpenMP spec mandates that it is an integer type in Fortran, i.e.
> integer(kind=omp_depend_kind).

I was focussing on getting the raw amdgcn toolchain toolchain to work 
again. I had forgotten about that little detail with Fortran. :-(

Is there another way we can fix this without implementing all of TImode 
support or tracking down every place in the middle-end that wants to use 
TImode without checking the optab?

> Combining the impl choice and the type requirements that means that
> on 64bit systems, this requires __int128 support, cf. commit
> https://gcc.gnu.org/g:8d0b2b33748014ee57973c1d7bc9fd7706bb3da9
> and https://gcc.gnu.org/PR96306
> 
> (Side note: The definition in OpenMP is bad - it should have been
> some opaque derived type but that's a mistake done in OpenMP 5.0.)

:-(

Andrew

Re: [committed] amdgcn: disable TImode

[Andrew Stubbs]

Indeed, libgomp fails to build:

configure: error: unsupported system, cannot find Fortran int kind=16, 
needed for omp_depend_kind

I've reverted the patch for now. We'll just have to put up with a lot of 
new test failures in the stand-alone toolchain so that the offload 
toolchain will at least build.

I suspect we'll see some real failures here soon though.

Andrew

On 07/05/2021 23:45, Andrew Stubbs wrote:
> On 07/05/2021 18:11, Tobias Burnus wrote:
>> On 07.05.21 18:35, Andrew Stubbs wrote:
>>
>>> TImode has always been a problem on amdgcn, and now it is causing many
>>> new test failures, so I'm disabling it.
>>
>> Does still still work with libgomp?
>>
>> The patch sounds as if it might cause problems, but on the other hand,
>> I assume you did test it? To recall:
>  >
>> The problem is that OpenMP's depobj as implemented in GCC has
>> sizeof() = 2*sizeof(void*) and is implemented as a two-element struct 
>> in C/C++.
>> But the OpenMP spec mandates that it is an integer type in Fortran, i.e.
>> integer(kind=omp_depend_kind).
> 
> I was focussing on getting the raw amdgcn toolchain toolchain to work 
> again. I had forgotten about that little detail with Fortran. :-(
> 
> Is there another way we can fix this without implementing all of TImode 
> support or tracking down every place in the middle-end that wants to use 
> TImode without checking the optab?
> 
>> Combining the impl choice and the type requirements that means that
>> on 64bit systems, this requires __int128 support, cf. commit
>> https://gcc.gnu.org/g:8d0b2b33748014ee57973c1d7bc9fd7706bb3da9
>> and https://gcc.gnu.org/PR96306
>>
>> (Side note: The definition in OpenMP is bad - it should have been
>> some opaque derived type but that's a mistake done in OpenMP 5.0.)
> 
> :-(
> 
> Andrew