public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, rs6000] Fix constraints issue in _mm_cvtss_si{32,64}
@ 2018-11-08 20:19 Bill Schmidt
  2018-11-08 22:51 ` Segher Boessenkool
  0 siblings, 1 reply; 2+ messages in thread
From: Bill Schmidt @ 2018-11-08 20:19 UTC (permalink / raw)
  To: GCC Patches; +Cc: Segher Boessenkool

Hi,

We recently discovered that GCC is getting lucky with register allocation of
some inline assembly code, despite invalid register constraints.  In these
two functions, a "wi" constraint (VSX valid for direct moves) was used for a 
temporary that, as written, is further constrained to be an FPR.  This patch
fixes the problem by introducing a separate temporary with an "f" constraint
and breaking the lifetime of the existing temporary.  The existing temporary
can now have a less onerous "wa" constraint as it is no longer used within a
direct move instruction.

Installed and tested on powerpc64le-linux-gnu with no regressions.  Is this
okay for trunk?

Thanks,
Bill


2018-11-08  Bill Schmidt  <wschmidt@linux.ibm.com>
	    Jinsong Ji  <jji@us.ibm.com>

	* config/rs6000/xmmintrin.h (_mm_cvtss_si32): Fix incorrect
	constraints by introducing a new temporary.
	(_mm_cvtss_si64): Likewise.


Index: gcc/config/rs6000/xmmintrin.h
===================================================================
--- gcc/config/rs6000/xmmintrin.h	(revision 265895)
+++ gcc/config/rs6000/xmmintrin.h	(working copy)
@@ -908,13 +908,15 @@ _mm_cvtss_si32 (__m128 __A)
   __m64 res = 0;
 #ifdef _ARCH_PWR8
   __m128 vtmp;
+  double dtmp;
   __asm__(
-      "xxsldwi %x1,%x2,%x2,3;\n"
-      "xscvspdp %x1,%x1;\n"
-      "fctiw  %1,%1;\n"
-      "mfvsrd  %0,%x1;\n"
+      "xxsldwi %x1,%x3,%x3,3;\n"
+      "xscvspdp %x2,%x1;\n"
+      "fctiw  %2,%2;\n"
+      "mfvsrd  %0,%x2;\n"
       : "=r" (res),
-	"=&wi" (vtmp)
+      	"=&wa" (vtmp),
+        "=f" (dtmp)
       : "wa" (__A)
       : );
 #else
@@ -939,13 +941,15 @@ _mm_cvtss_si64 (__m128 __A)
   __m64 res = 0;
 #ifdef _ARCH_PWR8
   __m128 vtmp;
+  double dtmp;
   __asm__(
-      "xxsldwi %x1,%x2,%x2,3;\n"
-      "xscvspdp %x1,%x1;\n"
-      "fctid  %1,%1;\n"
-      "mfvsrd  %0,%x1;\n"
+      "xxsldwi %x1,%x3,%x3,3;\n"
+      "xscvspdp %x2,%x1;\n"
+      "fctid  %2,%2;\n"
+      "mfvsrd  %0,%x2;\n"
       : "=r" (res),
-	"=&wi" (vtmp)
+        "=&wa" (vtmp),
+        "=f" (dtmp)
       : "wa" (__A)
       : );
 #else

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH, rs6000] Fix constraints issue in _mm_cvtss_si{32,64}
  2018-11-08 20:19 [PATCH, rs6000] Fix constraints issue in _mm_cvtss_si{32,64} Bill Schmidt
@ 2018-11-08 22:51 ` Segher Boessenkool
  0 siblings, 0 replies; 2+ messages in thread
From: Segher Boessenkool @ 2018-11-08 22:51 UTC (permalink / raw)
  To: Bill Schmidt; +Cc: GCC Patches

Hi!

On Thu, Nov 08, 2018 at 02:18:51PM -0600, Bill Schmidt wrote:
> We recently discovered that GCC is getting lucky with register allocation of
> some inline assembly code, despite invalid register constraints.  In these
> two functions, a "wi" constraint (VSX valid for direct moves) was used for a 

"wi" is not direct move only, that is "wj".  "wi" is for DImode in VSX.
It allows all VSX registers (or nothing at all).

> temporary that, as written, is further constrained to be an FPR.  This patch
> fixes the problem by introducing a separate temporary with an "f" constraint
> and breaking the lifetime of the existing temporary.  The existing temporary
> can now have a less onerous "wa" constraint as it is no longer used within a
> direct move instruction.

It could already use a "wa", and the "f" isn't needed at all?  But it
should use xsrdpic then, i.e. the VSX instruction instead of the FP insn.
Maybe that only works for 64 bit though (for the overflow behaviour).  Hrm.
Although it doesn't currently implement overflow behaviour correctly either!

Your code is not wrong though, so okay for trunk.  Thanks!


Segher

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-11-08 22:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-08 20:19 [PATCH, rs6000] Fix constraints issue in _mm_cvtss_si{32,64} Bill Schmidt
2018-11-08 22:51 ` Segher Boessenkool

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