On Fri, Dec 23, 2016 at 04:34:10PM -0600, Segher Boessenkool wrote:
> 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?
Actually, since the test case was written at the time I made the thinko, the
original test case tested if 12 was illegal instead of 13. I have adjusted the
test case. I also forgot to adjust the check in rs6000.c for the bounds being
0..12 instead of 0..11. I fixed that as well. This is the patch that was
checked in:
[gcc]
2016-12-27 Michael Meissner <meiss...@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/rs6000.c (altivec_expand_builtin): 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.
[gcc/testsuite]
2016-12-27 Michael Meissner <meiss...@linux.vnet.ibm.com>
* gcc.target/powerpc/p9-vinsert4b-2.c: Update test to test for 13
being out of bounds instead of 12.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/predicates.md
===================================================================
--- gcc/config/rs6000/predicates.md (revision 243943)
+++ 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.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 243943)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -15839,9 +15839,9 @@ altivec_expand_builtin (tree exp, rtx ta
if (arg1 == error_mark_node)
return expand_call (exp, target, false);
- if (TREE_CODE (arg1) != INTEGER_CST || TREE_INT_CST_LOW (arg1) > 11)
+ if (TREE_CODE (arg1) != INTEGER_CST || TREE_INT_CST_LOW (arg1) > 12)
{
- error ("second argument to vec_vextract4b must 0..11");
+ error ("second argument to vec_vextract4b must 0..12");
return expand_call (exp, target, false);
}
break;
@@ -15856,9 +15856,9 @@ altivec_expand_builtin (tree exp, rtx ta
if (arg2 == error_mark_node)
return expand_call (exp, target, false);
- if (TREE_CODE (arg2) != INTEGER_CST || TREE_INT_CST_LOW (arg2) > 11)
+ if (TREE_CODE (arg2) != INTEGER_CST || TREE_INT_CST_LOW (arg2) > 12)
{
- error ("third argument to vec_vinsert4b must 0..11");
+ error ("third argument to vec_vinsert4b must 0..12");
return expand_call (exp, target, false);
}
break;
Index: gcc/config/rs6000/vsx.md
===================================================================
--- gcc/config/rs6000/vsx.md (revision 243943)
+++ 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"
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md (revision 243943)
+++ 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/testsuite/gcc.target/powerpc/p9-vinsert4b-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/p9-vinsert4b-2.c (revision 243943)
+++ gcc/testsuite/gcc.target/powerpc/p9-vinsert4b-2.c (working copy)
@@ -8,7 +8,7 @@
vector signed char
ins_v4si (vector int vi, vector signed char vc)
{
- return vec_vinsert4b (vi, vc, 12); /* { dg-error "vec_vinsert4b" } */
+ return vec_vinsert4b (vi, vc, 13); /* { dg-error "vec_vinsert4b" } */
}
vector unsigned char
@@ -20,7 +20,7 @@ ins_di (long di, vector unsigned char vc
long
vext1 (vector signed char vc)
{
- return vec_vextract4b (vc, 12); /* { dg-error "vec_vextract4b" } */
+ return vec_vextract4b (vc, 13); /* { dg-error "vec_vextract4b" } */
}
long
