RE: [PATCH, MIPS] Calling convention differs depending on the presence of MSA

[Matthew Fortune]

Moore, Catherine  writes:
> > 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

[Moore, Catherine]

> -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

[Matthew Fortune]

Sameera Deshpande  writes:
> 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

[Sameera Deshpande]

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