Message ID | 16a39e7e-0328-da2f-f233-421d40e976a0@codesourcery.com |
---|---|
State | New |
Headers | show |
Series | [committed] amdgcn: disable TImode | expand |
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
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
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
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.