* [PATCH, MIPS] Calling convention differs depending on the presence of MSA
@ 2017-02-08 8:41 Sameera Deshpande
2017-02-12 4:19 ` Sameera Deshpande
0 siblings, 1 reply; 5+ messages in thread
From: Sameera Deshpande @ 2017-02-08 8:41 UTC (permalink / raw)
To: Matthew Fortune; +Cc: gcc-patches
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH, MIPS] Calling convention differs depending on the presence of MSA
2017-02-08 8:41 [PATCH, MIPS] Calling convention differs depending on the presence of MSA Sameera Deshpande
@ 2017-02-12 4:19 ` Sameera Deshpande
2017-02-21 10:44 ` Matthew Fortune
0 siblings, 1 reply; 5+ messages in thread
From: Sameera Deshpande @ 2017-02-12 4:19 UTC (permalink / raw)
To: Matthew Fortune; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 997 bytes --]
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.
[-- Attachment #2: fix_calling_convention.patch --]
[-- Type: application/octet-stream, Size: 1448 bytes --]
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index da7fa8f..158540a 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -6472,9 +6472,16 @@ 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));
+ machine_mode mode;
+ mode = TYPE_MODE (type);
+
+ if (VECTOR_FLOAT_TYPE_P (type))
+ return true;
+
+ if (TARGET_OLDABI)
+ return (mode == BLKmode);
+
+ return (!IN_RANGE (int_size_in_bytes (type), 0, 2 * UNITS_PER_WORD));
}
\f
/* 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 0000000..002a182
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/msa-fp-cc.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-mmsa" } */
+/* { dg-skip-if "Vectorization needed" { *-*-* } { "-O0" "-O1" "-O2"} { "" } } */
+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" } } */
+
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH, MIPS] Calling convention differs depending on the presence of MSA
2017-02-12 4:19 ` Sameera Deshpande
@ 2017-02-21 10:44 ` Matthew Fortune
2017-02-21 23:51 ` Moore, Catherine
0 siblings, 1 reply; 5+ messages in thread
From: Matthew Fortune @ 2017-02-21 10:44 UTC (permalink / raw)
To: Sameera Deshpande, Moore, Catherine (Catherine_Moore@mentor.com)
Cc: gcc-patches
Sameera Deshpande <Sameera.Deshpande@imgtec.com> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH, MIPS] Calling convention differs depending on the presence of MSA
2017-02-21 10:44 ` Matthew Fortune
@ 2017-02-21 23:51 ` Moore, Catherine
2017-02-22 23:44 ` Matthew Fortune
0 siblings, 1 reply; 5+ messages in thread
From: Moore, Catherine @ 2017-02-21 23:51 UTC (permalink / raw)
To: Matthew Fortune, Sameera Deshpande; +Cc: gcc-patches
> -----Original Message-----
> From: Matthew Fortune [mailto:Matthew.Fortune@imgtec.com]
> Sent: Tuesday, February 21, 2017 5:35 AM
> To: Sameera Deshpande <Sameera.Deshpande@imgtec.com>; Moore,
> Catherine <Catherine_Moore@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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH, MIPS] Calling convention differs depending on the presence of MSA
2017-02-21 23:51 ` Moore, Catherine
@ 2017-02-22 23:44 ` Matthew Fortune
0 siblings, 0 replies; 5+ messages in thread
From: Matthew Fortune @ 2017-02-22 23:44 UTC (permalink / raw)
To: Sameera Deshpande; +Cc: gcc-patches, Moore, Catherine
Moore, Catherine <Catherine_Moore@mentor.com> 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 0000000..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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-02-22 23:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-08 8:41 [PATCH, MIPS] Calling convention differs depending on the presence of MSA Sameera Deshpande
2017-02-12 4:19 ` Sameera Deshpande
2017-02-21 10:44 ` Matthew Fortune
2017-02-21 23:51 ` Moore, Catherine
2017-02-22 23:44 ` Matthew Fortune
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).