RE: [PATCH, MIPS] Calling convention differs depending on the presence of MSA
Moore, Catherinewrites: > > As this is an ABI fix, just wait a day or so in case Catherine has > > any comment otherwise OK to commit. > > > I have not further comments on this patch. Please commit. Hi Sameera, I've been considering this patch further and have realised that my review was preoccupied with o32 behaviour. The patch fixes o32 but breaks all other ABIs because it forces floating point vectors with size up to 2*word_size to be in memory after the change when they would have been in registers before. Since you mentioned off list you're having issues committing I've gone ahead and made the necessary changes and committed for you as below. Thanks for your work on this. Matthew gcc/ * config/mips/mips.c (mips_return_in_memory): Force FP vector types to be returned in memory for o32 ABI. gcc/testsuite/ * gcc.target/mips/msa-fp-cc.c: New test. --- diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 7974a16..4e13fbe 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -6472,9 +6472,13 @@ mips_function_value_regno_p (const unsigned int regno) static bool mips_return_in_memory (const_tree type, const_tree fndecl ATTRIBUTE_UNUSED) { - return (TARGET_OLDABI - ? TYPE_MODE (type) == BLKmode - : !IN_RANGE (int_size_in_bytes (type), 0, 2 * UNITS_PER_WORD)); + if (TARGET_OLDABI) +/* Ensure that any floating point vector types are returned via memory + even if they are supported through a vector mode with some ASEs. */ +return (VECTOR_FLOAT_TYPE_P (type) + || TYPE_MODE (type) == BLKmode); + + return (!IN_RANGE (int_size_in_bytes (type), 0, 2 * UNITS_PER_WORD)); } /* Implement TARGET_SETUP_INCOMING_VARARGS. */ diff --git a/gcc/testsuite/gcc.target/mips/msa-fp-cc.c b/gcc/testsuite/gcc.target/mips/msa-fp-cc.c new file mode 100644 index 000..3d293f3 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/msa-fp-cc.c @@ -0,0 +1,19 @@ +/* { dg-do compile } */ +/* { dg-options "-mabi=32 -mfp64 -mhard-float -mmsa" } */ +typedef float v4f32 __attribute__((vector_size(16))); +typedef double v2f64 __attribute__((vector_size(16))); + +v4f32 +fcmpOeqVector4 (v4f32 a, v4f32 b) +{ + return a + b; +} + +v2f64 +fcmpOeqVector2 (v2f64 a, v2f64 b) +{ + return a + b; +} + +/* { dg-final { scan-assembler-not "copy_s" } } */ +/* { dg-final { scan-assembler-not "insert" } } */ -- 2.2.1
RE: [PATCH, MIPS] Calling convention differs depending on the presence of MSA
> -Original Message- > From: Matthew Fortune [mailto:matthew.fort...@imgtec.com] > Sent: Tuesday, February 21, 2017 5:35 AM > To: Sameera Deshpande <sameera.deshpa...@imgtec.com>; Moore, > Catherine <catherine_mo...@mentor.com> > Cc: gcc-patches@gcc.gnu.org > Subject: RE: [PATCH, MIPS] Calling convention differs depending on the > presence of MSA > > > As this is an ABI fix, just wait a day or so in case Catherine has > any comment otherwise OK to commit. > I have not further comments on this patch. Please commit.
RE: [PATCH, MIPS] Calling convention differs depending on the presence of MSA
Sameera Deshpandewrites: > Hi Matthew, > > Please find attached updated patch as per our offline discussion. > > I have disabled return in registers for all vector float types, and > updated the test case accordingly. > > Ok for trunk? Hi Sameera, Sorry for the slow response. I'd like to explain my thinking here as I believe your change is right but you are actually ending up fixing more issues than you expected! There are 3 potential vector modes that are affected by your change rather than just 2. V2SF (paired single), V4SF and V2DF (MSA). The V4SF and V2DF changes are the ones needed for MSA as the test case shows. These become valid modes with -mmsa so the float vector types no longer get assigned BLKmode. (As an aside I think it is a legacy bug that this function was ever defined in terms of modes for o32). The V2SF is an interesting one as it would not historically have been a valid mode for o32 until o32 FP64 was re-invented. Now that o32 FP64 is a compatible ABI extension we(I) have in fact allowed an incompatible ABI change through as a vector of 2 floats will be returned in registers for o32 FP64 and not the other o32 variants. As an aside... The integer vector types up to 16-bytes get returned in registers also potentially by chance because... 1) We have TImode support in the backend 2) TImode is 16-bytes and hence the type->mode logic will fall back to looking for a wider integer mode for integer vectors and find TImode 3) The mips_hard_regno_mode_ok_p logic does not limit the size of a mode allowed in GPRs as long as the first register is even. 4) TImode is not BLKmode so is not forced to memory here. I'm doubtful this is by design but it is what it is. Let it hereby be part of the MIPS o32 ABI definition! Are you sure you need the dg-skip-if on the test case? Perhaps just -O0. The test does not need vectorization as it is explicitly Vectorized but it may break the scan-assembler at -O0. Also please can you apply the normal code style to the tests? v4f32 fcmpOeqVector4 (v4f32 a, v4f32 b) { return a + b; } As this is an ABI fix, just wait a day or so in case Catherine has any comment otherwise OK to commit. Thanks, Matthew > > - Thanks and regards, > Sameera D. > > From: Sameera Deshpande > Sent: 08 February 2017 14:10:52 > To: Matthew Fortune > Cc: gcc-patches@gcc.gnu.org > Subject: [PATCH, MIPS] Calling convention differs depending on the > presence of MSA > > Hi Matthew, > > Please find attached the patch to fix the calling convention issue, > where argument and result passing convention differed for MSA and non- > MSA variants. > > The implementation of TARGET_RETURN_IN_MEMORY is altered to block V4SF > to be returned in registers. > > Ok for trunk? > > - Thanks and regards, > Sameera D. > > > Changelog: > gcc/ > * config/mips/mips.c (mips_return_in_memory) : Restrict V4SFmode to > be returned in registers. > > gcc/testsuite/ > * gcc.target/mips/msa-fp-cc.c : New testcase.
RE: [PATCH, MIPS] Calling convention differs depending on the presence of MSA
Hi Matthew, Please find attached updated patch as per our offline discussion. I have disabled return in registers for all vector float types, and updated the test case accordingly. Ok for trunk? - Thanks and regards, Sameera D. From: Sameera Deshpande Sent: 08 February 2017 14:10:52 To: Matthew Fortune Cc: gcc-patches@gcc.gnu.org Subject: [PATCH, MIPS] Calling convention differs depending on the presence of MSA Hi Matthew, Please find attached the patch to fix the calling convention issue, where argument and result passing convention differed for MSA and non-MSA variants. The implementation of TARGET_RETURN_IN_MEMORY is altered to block V4SF to be returned in registers. Ok for trunk? - Thanks and regards, Sameera D. Changelog: gcc/ * config/mips/mips.c (mips_return_in_memory) : Restrict V4SFmode to be returned in registers. gcc/testsuite/ * gcc.target/mips/msa-fp-cc.c : New testcase. fix_calling_convention.patch Description: fix_calling_convention.patch