diff mbox

, Fix PowerPC ISA 3.0 word extract/insert thinkos

Message ID 20161223214721.GA9223@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner Dec. 23, 2016, 9:47 p.m. UTC 
I had two thinkos in my previous patches for ISA 3.0 (power9) support that both
relate to word extraction and insertion.

The first thinko was that I thought the index for the first byte in the 4 bytes
to be extracted should be 0..11, when it should be 0..12.  If it isn't allowed
to be 12, you cannot extract the 32-bit word at the bottom of the vector
register.

The second thinko is where I was doing zeo extending of a 32-bit value within
a vector register, I used xxextractuw with a byte offset of 1 instead of 4.

I have done the usual bootstrap and make check with no regressions on these
patches.  Can I install them into the trunk?

2016-12-23  Michael Meissner  <meissner@linux.vnet.ibm.com>

	* config/rs6000/predicates.md (const_0_to_12_operand): Rename
	predicate and change test from 0..11 to 0..12 to match the
	semantics of the word extract/insert instructions.  Change all
	callers.
	(const_0_to_11_operand): Likewise.
	* config/rs6000/vsx.md (vextract4b): Likewise.
	(vextract4b_internal): Likewise.
	(vinsert4b): Likewise.
	(vinsert4b_internal): Likewise.
	(vinsert4b_di): Likewise.
	(vinsert4b_di_internal): Likewise.
	* config/rs6000/rs6000.md (zero_extendsi<mode>2): Fix offset used
	in xxextractuw to zero extend the word in the vector registers.
	(lfiwzx): Likewise.

Comments

Segher Boessenkool Dec. 23, 2016, 10:34 p.m. UTC | #1 
On Fri, Dec 23, 2016 at 04:47:22PM -0500, Michael Meissner wrote:
> I had two thinkos in my previous patches for ISA 3.0 (power9) support that both
> relate to word extraction and insertion.
> 
> The first thinko was that I thought the index for the first byte in the 4 bytes
> to be extracted should be 0..11, when it should be 0..12.  If it isn't allowed
> to be 12, you cannot extract the 32-bit word at the bottom of the vector
> register.
> 
> The second thinko is where I was doing zeo extending of a 32-bit value within
> a vector register, I used xxextractuw with a byte offset of 1 instead of 4.
> 
> I have done the usual bootstrap and make check with no regressions on these
> patches.  Can I install them into the trunk?

Yes please.  It sounds like we need a few more testcases though?


Segher


> 2016-12-23  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	* config/rs6000/predicates.md (const_0_to_12_operand): Rename
> 	predicate and change test from 0..11 to 0..12 to match the
> 	semantics of the word extract/insert instructions.  Change all
> 	callers.
> 	(const_0_to_11_operand): Likewise.
> 	* config/rs6000/vsx.md (vextract4b): Likewise.
> 	(vextract4b_internal): Likewise.
> 	(vinsert4b): Likewise.
> 	(vinsert4b_internal): Likewise.
> 	(vinsert4b_di): Likewise.
> 	(vinsert4b_di_internal): Likewise.
> 	* config/rs6000/rs6000.md (zero_extendsi<mode>2): Fix offset used
> 	in xxextractuw to zero extend the word in the vector registers.
> 	(lfiwzx): Likewise.
diff mbox

Patch

Index: gcc/config/rs6000/predicates.md
===================================================================
--- gcc/config/rs6000/predicates.md	(revision 243891)
+++ gcc/config/rs6000/predicates.md	(working copy)
@@ -211,9 +211,9 @@  (define_predicate "const_0_to_7_operand"
        (match_test "IN_RANGE (INTVAL (op), 0, 7)")))
 
 ;; Match op = 0..11
-(define_predicate "const_0_to_11_operand"
+(define_predicate "const_0_to_12_operand"
   (and (match_code "const_int")
-       (match_test "IN_RANGE (INTVAL (op), 0, 11)")))
+       (match_test "IN_RANGE (INTVAL (op), 0, 12)")))
 
 ;; Match op = 0..15
 (define_predicate "const_0_to_15_operand"
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 243891)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -855,7 +855,7 @@  (define_insn "zero_extendsi<mode>2"
    lxsiwzx %x0,%y1
    mtvsrwz %x0,%1
    mfvsrwz %0,%x1
-   xxextractuw %x0,%x1,1"
+   xxextractuw %x0,%x1,4"
   [(set_attr "type" "load,shift,fpload,fpload,mffgpr,mftgpr,vecexts")])
 
 (define_insn_and_split "*zero_extendsi<mode>2_dot"
@@ -5131,7 +5131,7 @@  (define_insn "lfiwzx"
    lfiwzx %0,%y1
    lxsiwzx %x0,%y1
    mtvsrwz %x0,%1
-   xxextractuw %x0,%x1,1"
+   xxextractuw %x0,%x1,4"
   [(set_attr "type" "fpload,fpload,mftgpr,vecexts")])
 
 (define_insn_and_split "floatunssi<mode>2_lfiwzx"
Index: gcc/config/rs6000/vsx.md
===================================================================
--- gcc/config/rs6000/vsx.md	(revision 243891)
+++ gcc/config/rs6000/vsx.md	(working copy)
@@ -3813,7 +3813,7 @@  (define_insn "vextuwrx"
 (define_expand "vextract4b"
   [(set (match_operand:DI 0 "gpc_reg_operand")
 	(unspec:DI [(match_operand:V16QI 1 "vsx_register_operand")
-		    (match_operand:QI 2 "const_0_to_11_operand")]
+		    (match_operand:QI 2 "const_0_to_12_operand")]
 		   UNSPEC_XXEXTRACTUW))]
   "TARGET_P9_VECTOR"
 {
@@ -3824,7 +3824,7 @@  (define_expand "vextract4b"
 (define_insn_and_split "*vextract4b_internal"
   [(set (match_operand:DI 0 "gpc_reg_operand" "=wj,r")
 	(unspec:DI [(match_operand:V16QI 1 "vsx_register_operand" "wa,v")
-		    (match_operand:QI 2 "const_0_to_11_operand" "n,n")]
+		    (match_operand:QI 2 "const_0_to_12_operand" "n,n")]
 		   UNSPEC_XXEXTRACTUW))]
   "TARGET_P9_VECTOR"
   "@
@@ -3852,7 +3852,7 @@  (define_expand "vinsert4b"
   [(set (match_operand:V16QI 0 "vsx_register_operand")
 	(unspec:V16QI [(match_operand:V4SI 1 "vsx_register_operand")
 		       (match_operand:V16QI 2 "vsx_register_operand")
-		       (match_operand:QI 3 "const_0_to_11_operand")]
+		       (match_operand:QI 3 "const_0_to_12_operand")]
 		   UNSPEC_XXINSERTW))]
   "TARGET_P9_VECTOR"
 {
@@ -3870,7 +3870,7 @@  (define_insn "*vinsert4b_internal"
   [(set (match_operand:V16QI 0 "vsx_register_operand" "=wa")
 	(unspec:V16QI [(match_operand:V4SI 1 "vsx_register_operand" "wa")
 		       (match_operand:V16QI 2 "vsx_register_operand" "0")
-		       (match_operand:QI 3 "const_0_to_11_operand" "n")]
+		       (match_operand:QI 3 "const_0_to_12_operand" "n")]
 		   UNSPEC_XXINSERTW))]
   "TARGET_P9_VECTOR"
   "xxinsertw %x0,%x1,%3"
@@ -3880,7 +3880,7 @@  (define_expand "vinsert4b_di"
   [(set (match_operand:V16QI 0 "vsx_register_operand")
 	(unspec:V16QI [(match_operand:DI 1 "vsx_register_operand")
 		       (match_operand:V16QI 2 "vsx_register_operand")
-		       (match_operand:QI 3 "const_0_to_11_operand")]
+		       (match_operand:QI 3 "const_0_to_12_operand")]
 		   UNSPEC_XXINSERTW))]
   "TARGET_P9_VECTOR"
 {
@@ -3892,7 +3892,7 @@  (define_insn "*vinsert4b_di_internal"
   [(set (match_operand:V16QI 0 "vsx_register_operand" "=wa")
 	(unspec:V16QI [(match_operand:DI 1 "vsx_register_operand" "wj")
 		       (match_operand:V16QI 2 "vsx_register_operand" "0")
-		       (match_operand:QI 3 "const_0_to_11_operand" "n")]
+		       (match_operand:QI 3 "const_0_to_12_operand" "n")]
 		   UNSPEC_XXINSERTW))]
   "TARGET_P9_VECTOR"
   "xxinsertw %x0,%x1,%3"