diff mbox

[i386,1/8,AVX512] Adjust register classes.

Message ID 20130822185627.GB3556@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Kirill Yukhin Aug. 22, 2013, 6:56 p.m. UTC 
Hello,

On 21 Aug 13:02, Richard Henderson wrote:
> On 08/21/2013 11:28 AM, Kirill Yukhin wrote:
> >  	    (eq_attr "alternative" "12,13")
> > -	      (cond [(ior (not (match_test "TARGET_SSE2"))
> > +	      (cond [(ior (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[0]))")
> > +			  (and (match_test "REG_P (operands[1])")
> > +			       (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[1]))")))
> > +		       (const_string "XI")
> > +		     (ior (not (match_test "TARGET_SSE2"))
> >  			  (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL"))
> >  		       (const_string "V4SF")
> >  		     (match_test "TARGET_AVX")
> 
> Better.  And while it produces the correct results, using match_operand would
> be better than embedding a reference to operands within a match_test.

In order to get rid of direct references to operands in attrs
of scalar mov*_internal I've introduced new predicate and use it with
match_operand instead.

ChangeLog:
2013-08-22  Kirill Yukhin  <kirill.yukhin@intel.com>

	* gcc/config/i386/i386.md (*movti_internal): Use
	predicate to determine if EVEX is needed.
	(*movsi_internal): Ditto.
	(*movdf_internal): Ditto.
	(*movsf_internal): Ditto.
	* gcc/config/i386/mmx.md (*mov<mode>_internal): Ditto.

Testing:
  1. Bootstrap pass.
  2. make check shows no regressions.
  3. Spec 2000 & 2006 build show no regressions both with and without -mavx512f option.
  4. Spec 2000 & 2006 run shows no stability regressions without -mavx512f option.

Is it ok to install to trunk?

--
Thanks, K

---
 gcc/config/i386/i386.md       | 20 ++++++++------------
 gcc/config/i386/mmx.md        |  5 ++---
 gcc/config/i386/predicates.md |  6 ++++++
 3 files changed, 16 insertions(+), 15 deletions(-)

Comments

Richard Henderson Aug. 22, 2013, 7:06 p.m. UTC | #1 
On 08/22/2013 11:56 AM, Kirill Yukhin wrote:
> ChangeLog:
> 2013-08-22  Kirill Yukhin  <kirill.yukhin@intel.com>
> 
> 	* gcc/config/i386/i386.md (*movti_internal): Use
> 	predicate to determine if EVEX is needed.
> 	(*movsi_internal): Ditto.
> 	(*movdf_internal): Ditto.
> 	(*movsf_internal): Ditto.
> 	* gcc/config/i386/mmx.md (*mov<mode>_internal): Ditto.


Ok.


r~
H.J. Lu Aug. 22, 2013, 7:08 p.m. UTC | #2 
On Thu, Aug 22, 2013 at 12:06 PM, Richard Henderson <rth@redhat.com> wrote:
> On 08/22/2013 11:56 AM, Kirill Yukhin wrote:
>> ChangeLog:
>> 2013-08-22  Kirill Yukhin  <kirill.yukhin@intel.com>
>>
>>       * gcc/config/i386/i386.md (*movti_internal): Use
>>       predicate to determine if EVEX is needed.
>>       (*movsi_internal): Ditto.
>>       (*movdf_internal): Ditto.
>>       (*movsf_internal): Ditto.
>>       * gcc/config/i386/mmx.md (*mov<mode>_internal): Ditto.
>
>
> Ok.
>
>
> r~

ChangeLog entry for ext_sse_reg_operand is missing.
Kirill Yukhin Aug. 23, 2013, 7:35 a.m. UTC | #3 
Hello,

On 22 Aug 12:06, Richard Henderson wrote:
> Ok.

I've updated ChangeLog (thanks, HJ!) and
checked in to main trunk:  http://gcc.gnu.org/ml/gcc-cvs/2013-08/msg00545.html

--
Thanks, K
Iain Sandoe Sept. 1, 2013, 9:36 a.m. UTC | #4 
Hi Kirill,

On 23 Aug 2013, at 08:35, Kirill Yukhin wrote:

> On 22 Aug 12:06, Richard Henderson wrote:
>> Ok.
> 
> I've updated ChangeLog (thanks, HJ!) and
> checked in to main trunk:  http://gcc.gnu.org/ml/gcc-cvs/2013-08/msg00545.html

This patch [actually the change at 201915] also broke X86 Darwin bootstrap/ABI: pr59269
- ISTM that SSE_REGNO_P() now returns true for a different set of registers than before the patch,
I've attached a starting-point to fix to the PR - but would welcome any additional inputs folks might have on how best to audit this change.

cheers
Iain
Kirill Yukhin Sept. 1, 2013, 4:41 p.m. UTC | #5 
Hello,
> This patch [actually the change at 201915] also broke X86 Darwin bootstrap/ABI: pr59269
> - ISTM that SSE_REGNO_P() now returns true for a different set of registers than before the patch,
> I've attached a starting-point to fix to the PR - but would welcome any additional inputs folks might have on how best to audit this change.
Correct bug id: pr58269
I'll take a look, thanks!

--
Thanks, K
diff mbox

Patch

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index b55fd6f..3d7533a 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -2059,9 +2059,8 @@ 
      (cond [(eq_attr "alternative" "2")
 	      (const_string "SI")
 	    (eq_attr "alternative" "12,13")
-	      (cond [(ior (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[0]))")
-			  (and (match_test "REG_P (operands[1])")
-			       (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[1]))")))
+	      (cond [(ior (match_operand 0 "ext_sse_reg_operand")
+			  (match_operand 1 "ext_sse_reg_operand"))
 		       (const_string "XI")
 		     (ior (not (match_test "TARGET_SSE2"))
 			  (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL"))
@@ -2192,9 +2191,8 @@ 
      (cond [(eq_attr "alternative" "2,3")
 	      (const_string "DI")
 	    (eq_attr "alternative" "6,7")
-	      (cond [(ior (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[0]))")
-			  (and (match_test "REG_P (operands[1])")
-			       (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[1]))")))
+	      (cond [(ior (match_operand 0 "ext_sse_reg_operand")
+			  (match_operand 1 "ext_sse_reg_operand"))
 		       (const_string "XI")
 		     (ior (not (match_test "TARGET_SSE2"))
 			  (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL"))
@@ -2923,9 +2921,8 @@ 
 
 	       /* movaps is one byte shorter for non-AVX targets.  */
 	       (eq_attr "alternative" "10,14")
-		 (cond [(ior (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[0]))")
-			     (and (match_test "REG_P (operands[1])")
-				  (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[1]))")))
+		 (cond [(ior (match_operand 0 "ext_sse_reg_operand")
+			     (match_operand 1 "ext_sse_reg_operand"))
 			  (const_string "V8DF")
 			(ior (not (match_test "TARGET_SSE2"))
 			     (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL"))
@@ -3072,9 +3069,8 @@ 
 		  better to maintain the whole registers in single format
 		  to avoid problems on using packed logical operations.  */
 	       (eq_attr "alternative" "6")
-		 (cond [(ior (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[0]))")
-			     (and (match_test "REG_P (operands[1])")
-				  (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[1]))")))
+		 (cond [(ior  (match_operand 0 "ext_sse_reg_operand")
+			      (match_operand 1 "ext_sse_reg_operand"))
 			  (const_string "V16SF")
 			(ior (match_test "TARGET_SSE_PARTIAL_REG_DEPENDENCY")
 			     (match_test "TARGET_SSE_SPLIT_REGS"))
diff --git a/gcc/config/i386/mmx.md b/gcc/config/i386/mmx.md
index 681cdb7..17e2499 100644
--- a/gcc/config/i386/mmx.md
+++ b/gcc/config/i386/mmx.md
@@ -185,9 +185,8 @@ 
      (cond [(eq_attr "alternative" "2")
 	      (const_string "SI")
 	    (eq_attr "alternative" "11,12,15,16")
-	      (cond [(ior (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[0]))")
-			  (and (match_test "REG_P (operands[1])")
-			       (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[1]))")))
+	      (cond [(ior (match_operand 0 "ext_sse_reg_operand")
+			  (match_operand 1 "ext_sse_reg_operand"))
 			(const_string "XI")
 		     (match_test "<MODE>mode == V2SFmode")
 		       (const_string "V4SF")
diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index b64ef69..3959c38 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -47,6 +47,12 @@ 
   (and (match_code "reg")
        (match_test "SSE_REGNO_P (REGNO (op))")))
 
+;; True if the operand is an AVX-512 new register.
+(define_predicate "ext_sse_reg_operand"
+  (and (match_code "reg")
+       (match_test "EXT_REX_SSE_REGNO_P (REGNO (op))")))
+
+
 ;; True if the operand is a Q_REGS class register.
 (define_predicate "q_regs_operand"
   (match_operand 0 "register_operand")