Hi,

On 2021/5/13 18:49, Segher Boessenkool wrote:
Hi!

On Fri, Apr 30, 2021 at 01:32:58AM -0500, Xionghu Luo wrote:
The vsel instruction is a bit-wise select instruction.  Using an
IF_THEN_ELSE to express it in RTL is wrong and leads to wrong code
being generated in the combine pass.  Per element selection is a
subset of per bit-wise selection,with the patch the pattern is
written using bit operations.  But there are 8 different patterns
to define "op0 := (op1 & ~op3) | (op2 & op3)":

(~op3&op1) | (op3&op2),
(~op3&op1) | (op2&op3),
(op3&op2) | (~op3&op1),
(op2&op3) | (~op3&op1),
(op1&~op3) | (op3&op2),
(op1&~op3) | (op2&op3),
(op3&op2) | (op1&~op3),
(op2&op3) | (op1&~op3),

Combine pass will swap (op1&~op3) to (~op3&op1) due to commutative
canonical, which could reduce it to the FIRST 4 patterns, but it won't
swap (op2&op3) | (~op3&op1) to (~op3&op1) | (op2&op3), so this patch
handles it with two patterns with different NOT op3 position and check
equality inside it.

Yup, that latter case does not have canonicalisation rules.  Btw, not
only combine does this canonicalisation: everything should,
non-canonical RTL is invalid RTL (in the instruction stream, you can do
everything in temporary code of course, as long as the RTL isn't
malformed).

-(define_insn "*altivec_vsel<mode>"
+(define_insn "altivec_vsel<mode>"
    [(set (match_operand:VM 0 "altivec_register_operand" "=v")
-       (if_then_else:VM
-        (ne:CC (match_operand:VM 1 "altivec_register_operand" "v")
-               (match_operand:VM 4 "zero_constant" ""))
-        (match_operand:VM 2 "altivec_register_operand" "v")
-        (match_operand:VM 3 "altivec_register_operand" "v")))]
-  "VECTOR_MEM_ALTIVEC_P (<MODE>mode)"
-  "vsel %0,%3,%2,%1"
+       (ior:VM
+        (and:VM
+         (not:VM (match_operand:VM 3 "altivec_register_operand" "v"))
+         (match_operand:VM 1 "altivec_register_operand" "v"))
+        (and:VM
+         (match_operand:VM 2 "altivec_register_operand" "v")
+         (match_operand:VM 4 "altivec_register_operand" "v"))))]
+  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)
+  && (rtx_equal_p (operands[2], operands[3])
+  || rtx_equal_p (operands[4], operands[3]))"
+  {
+    if (rtx_equal_p (operands[2], operands[3]))
+      return "vsel %0,%1,%4,%3";
+    else
+      return "vsel %0,%1,%2,%3";
+  }
    [(set_attr "type" "vecmove")])

That rtx_equal_p stuff is nice and tricky, but it is a bit too tricky I
think.  So please write this as two patterns (and keep the expand if
that helps).

I was a bit concerned that there would be a lot of duplicate code if we
write two patterns for each vsel, totally 4 similar patterns in
altivec.md and another 4 in vsx.md make it difficult to maintain, however
I updated it since you prefer this way, as you pointed out the xxsel in
vsx.md could be folded by later patch.


+(define_insn "altivec_vsel<mode>2"

(same here of course).

  ;; Fused multiply add.
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index f5676255387..d65bdc01055 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -3362,11 +3362,11 @@ const struct altivec_builtin_types 
altivec_overloaded_builtins[] = {
      RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_V2DI, 
RS6000_BTI_unsigned_V2DI },
    { ALTIVEC_BUILTIN_VEC_SEL, ALTIVEC_BUILTIN_VSEL_2DI,
      RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_V2DI },
-  { ALTIVEC_BUILTIN_VEC_SEL, ALTIVEC_BUILTIN_VSEL_2DI,
+  { ALTIVEC_BUILTIN_VEC_SEL, ALTIVEC_BUILTIN_VSEL_2DI_UNS,

Are the _uns things still used for anything?  But, let's not change
this until Bill's stuff is in :-)

Why do you want to change this here, btw?  I don't understand.

OK, they are actually "unsigned type" overload builtin functions, change
it or not so far won't cause functionality issue, I will revert this change
in the updated patch.


+  if (target == 0
+      || GET_MODE (target) != tmode
+      || ! (*insn_data[icode].operand[0].predicate) (target, tmode))

No space after ! and other unary operators (except for casts and other
operators you write with alphanumerics, like "sizeof").  I know you
copied this code, but :-)

OK, thanks.


@@ -15608,8 +15606,6 @@ rs6000_emit_vector_cond_expr (rtx dest, rtx op_true, 
rtx op_false,
      case GEU:
      case LTU:
      case LEU:
-      /* Mark unsigned tests with CCUNSmode.  */
-      cc_mode = CCUNSmode;
/* Invert condition to avoid compound test if necessary. */
        if (rcode == GEU || rcode == LEU)

So this is related to the _uns thing.  Could you split off that change?
Probably as an earlier patch (but either works for me).

Not related to the ALTIVEC_BUILTIN_VSEL_2DI_UNS things, previously cc_mode
is a parameter to generate the condition for IF_THEN_ELSE instruction, now
we don't need it again as we use IOR (AND... AND...) style, remove it to avoid
build error.


-  cond2 = gen_rtx_fmt_ee (NE, cc_mode, gen_lowpart (dest_mode, mask),
-                         CONST0_RTX (dest_mode));
-  emit_insn (gen_rtx_SET (dest,
-                         gen_rtx_IF_THEN_ELSE (dest_mode,
-                                               cond2,
-                                               op_true,
-                                               op_false)));
+  rtx tmp = gen_rtx_IOR (dest_mode,
+                        gen_rtx_AND (dest_mode, gen_rtx_NOT (dest_mode, mask),
+                                     op_false),
+                        gen_rtx_AND (dest_mode, mask, op_true));
+  emit_insn (gen_rtx_SET (dest, tmp));



@@ -15629,6 +15625,9 @@ rs6000_emit_vector_cond_expr (rtx dest, rtx op_true, 
rtx op_false,
    if (!mask)
      return 0;
+ if (mask_mode != dest_mode)
+      mask = simplify_gen_subreg (dest_mode, mask, mask_mode, 0);

Indent just two characters please: line continuations (usually) align,
but indents do not.>
Can you fold vsel and xxsel together completely?  They have exactly the
same semantics!  This does not have to be in this patch of course.

I noticed that vperm/xxperm are folded together, do you mean fold vsel/xxsel
like them?  It's attached as:
0002-rs6000-Fold-xxsel-to-vsel-since-they-have-same-seman.patch
Thanks,
Xionghu
From 68cb56305caea6aaee083d814fd4d11590cbc766 Mon Sep 17 00:00:00 2001
From: Xionghu Luo <luo...@linux.ibm.com>
Date: Tue, 27 Apr 2021 01:07:25 -0500
Subject: [PATCH 1/2] rs6000: Fix wrong code generation for vec_sel [PR94613]

The vsel instruction is a bit-wise select instruction.  Using an
IF_THEN_ELSE to express it in RTL is wrong and leads to wrong code
being generated in the combine pass.  Per element selection is a
subset of per bit-wise selection,with the patch the pattern is
written using bit operations.  But there are 8 different patterns
to define "op0 := (op1 & ~op3) | (op2 & op3)":

(~op3&op1) | (op3&op2),
(~op3&op1) | (op2&op3),
(op3&op2) | (~op3&op1),
(op2&op3) | (~op3&op1),
(op1&~op3) | (op3&op2),
(op1&~op3) | (op2&op3),
(op3&op2) | (op1&~op3),
(op2&op3) | (op1&~op3),

Combine pass will swap (op1&~op3) to (~op3&op1) due to commutative
canonical, which could reduce it to the FIRST 4 patterns, but it won't
swap (op2&op3) | (~op3&op1) to (~op3&op1) | (op2&op3), so this patch
handles it with two patterns with different NOT op3 position and check
equality inside it.

Tested pass on Power8LE, any comments?

gcc/ChangeLog:

        * config/rs6000/altivec.md (*altivec_vsel<mode>): Change to ...
        (altivec_vsel<mode>): ... this and update define.
        (*altivec_vsel<mode>_uns): Delete.
        (altivec_vsel<mode>2): New define_insn.
        (altivec_vsel<mode>3): Likewise.
        (altivec_vsel<mode>4): Likewise.
        * config/rs6000/rs6000-call.c (altivec_expand_vec_sel_builtin):
        New.
        (altivec_expand_builtin): Call altivec_expand_vec_sel_builtin to
        expand vel_sel.
        * config/rs6000/rs6000.c (rs6000_emit_vector_cond_expr): Use
        bit-wise selection instead of per element.
        * config/rs6000/vector.md:
        * config/rs6000/vsx.md (*vsx_xxsel<mode>): Change to ...
        (vsx_xxsel<mode>): ... this and update define.
        (*vsx_xxsel<mode>_uns): Delete.
        (vsx_xxsel<mode>2): New define_insn.
        (vsx_xxsel<mode>3): Likewise.
        (vsx_xxsel<mode>4): Likewise.

gcc/testsuite/ChangeLog:

        * gcc.target/powerpc/pr94613.c: New test.
---
 gcc/config/rs6000/altivec.md               | 66 ++++++++++++++++-----
 gcc/config/rs6000/rs6000-call.c            | 62 ++++++++++++++++++++
 gcc/config/rs6000/rs6000.c                 | 19 +++---
 gcc/config/rs6000/vector.md                | 26 ++++-----
 gcc/config/rs6000/vsx.md                   | 68 +++++++++++++++++-----
 gcc/testsuite/gcc.target/powerpc/pr94613.c | 38 ++++++++++++
 6 files changed, 222 insertions(+), 57 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pr94613.c

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index 1351dafbc41..df8f24701d0 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -667,26 +667,60 @@ (define_insn "*altivec_gev4sf"
   "vcmpgefp %0,%1,%2"
   [(set_attr "type" "veccmp")])
 
-(define_insn "*altivec_vsel<mode>"
+(define_insn "altivec_vsel<mode>"
   [(set (match_operand:VM 0 "altivec_register_operand" "=v")
-       (if_then_else:VM
-        (ne:CC (match_operand:VM 1 "altivec_register_operand" "v")
-               (match_operand:VM 4 "zero_constant" ""))
-        (match_operand:VM 2 "altivec_register_operand" "v")
-        (match_operand:VM 3 "altivec_register_operand" "v")))]
-  "VECTOR_MEM_ALTIVEC_P (<MODE>mode)"
-  "vsel %0,%3,%2,%1"
+       (ior:VM
+        (and:VM
+         (not:VM (match_operand:VM 3 "altivec_register_operand" "v"))
+         (match_operand:VM 1 "altivec_register_operand" "v"))
+        (and:VM
+         (match_operand:VM 2 "altivec_register_operand" "v")
+         (match_operand:VM 4 "altivec_register_operand" "v"))))]
+  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)
+  && rtx_equal_p (operands[2], operands[3])"
+  "vsel %0,%1,%4,%3"
   [(set_attr "type" "vecmove")])
 
-(define_insn "*altivec_vsel<mode>_uns"
+(define_insn "altivec_vsel<mode>2"
   [(set (match_operand:VM 0 "altivec_register_operand" "=v")
-       (if_then_else:VM
-        (ne:CCUNS (match_operand:VM 1 "altivec_register_operand" "v")
-                  (match_operand:VM 4 "zero_constant" ""))
-        (match_operand:VM 2 "altivec_register_operand" "v")
-        (match_operand:VM 3 "altivec_register_operand" "v")))]
-  "VECTOR_MEM_ALTIVEC_P (<MODE>mode)"
-  "vsel %0,%3,%2,%1"
+       (ior:VM
+        (and:VM
+         (not:VM (match_operand:VM 3 "altivec_register_operand" "v"))
+         (match_operand:VM 1 "altivec_register_operand" "v"))
+        (and:VM
+         (match_operand:VM 2 "altivec_register_operand" "v")
+         (match_operand:VM 4 "altivec_register_operand" "v"))))]
+  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)
+  && rtx_equal_p (operands[4], operands[3])"
+  "vsel %0,%1,%2,%3"
+  [(set_attr "type" "vecmove")])
+
+(define_insn "altivec_vsel<mode>3"
+  [(set (match_operand:VM 0 "altivec_register_operand" "=v")
+       (ior:VM
+        (and:VM
+         (match_operand:VM 1 "altivec_register_operand" "v")
+         (match_operand:VM 2 "altivec_register_operand" "v"))
+        (and:VM
+         (not:VM (match_operand:VM 3 "altivec_register_operand" "v"))
+         (match_operand:VM 4 "altivec_register_operand" "v"))))]
+  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)
+  && rtx_equal_p (operands[1], operands[3])"
+  "vsel %0,%4,%2,%3"
+  [(set_attr "type" "vecmove")])
+
+(define_insn "altivec_vsel<mode>4"
+  [(set (match_operand:VM 0 "altivec_register_operand" "=v")
+       (ior:VM
+        (and:VM
+         (match_operand:VM 1 "altivec_register_operand" "v")
+         (match_operand:VM 2 "altivec_register_operand" "v"))
+        (and:VM
+         (not:VM (match_operand:VM 3 "altivec_register_operand" "v"))
+         (match_operand:VM 4 "altivec_register_operand" "v"))))]
+  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)
+  && rtx_equal_p (operands[2], operands[3])"
+  "vsel %0,%4,%1,%3"
   [(set_attr "type" "vecmove")])
 
 ;; Fused multiply add.
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index f5676255387..94593be0d2e 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -10996,6 +10996,45 @@ altivec_expand_vec_ext_builtin (tree exp, rtx target)
   return target;
 }
 
+/* Expand vec_sel builtin.  */
+static rtx
+altivec_expand_vec_sel_builtin (enum insn_code icode, tree exp, rtx target)
+{
+  rtx op0, op1, op2, pat;
+  tree arg0, arg1, arg2;
+
+  arg0 = CALL_EXPR_ARG (exp, 0);
+  op0 = expand_normal (arg0);
+  arg1 = CALL_EXPR_ARG (exp, 1);
+  op1 = expand_normal (arg1);
+  arg2 = CALL_EXPR_ARG (exp, 2);
+  op2 = expand_normal (arg2);
+
+  machine_mode tmode = insn_data[icode].operand[0].mode;
+  machine_mode mode0 = insn_data[icode].operand[1].mode;
+  machine_mode mode1 = insn_data[icode].operand[2].mode;
+  machine_mode mode2 = insn_data[icode].operand[3].mode;
+
+  if (target == 0 || GET_MODE (target) != tmode
+      || !(*insn_data[icode].operand[0].predicate) (target, tmode))
+    target = gen_reg_rtx (tmode);
+
+  if (!(*insn_data[icode].operand[1].predicate) (op0, mode0))
+    op0 = copy_to_mode_reg (mode0, op0);
+  if (!(*insn_data[icode].operand[2].predicate) (op1, mode1))
+    op1 = copy_to_mode_reg (mode1, op1);
+  if (!(*insn_data[icode].operand[3].predicate) (op2, mode2))
+    op2 = copy_to_mode_reg (mode2, op2);
+
+  pat = GEN_FCN (icode) (target, op0, op1, op2, op2);
+  if (pat)
+    emit_insn (pat);
+  else
+    return NULL_RTX;
+
+  return target;
+}
+
 /* Expand the builtin in EXP and store the result in TARGET.  Store
    true in *EXPANDEDP if we found a builtin to expand.  */
 static rtx
@@ -11181,6 +11220,29 @@ altivec_expand_builtin (tree exp, rtx target, bool 
*expandedp)
        emit_insn (pat);
       return NULL_RTX;
 
+    case ALTIVEC_BUILTIN_VSEL_2DF:
+      return altivec_expand_vec_sel_builtin (CODE_FOR_altivec_vselv2df, exp,
+                                            target);
+    case ALTIVEC_BUILTIN_VSEL_2DI:
+    case ALTIVEC_BUILTIN_VSEL_2DI_UNS:
+      return altivec_expand_vec_sel_builtin (CODE_FOR_altivec_vselv2di, exp,
+                                            target);
+    case ALTIVEC_BUILTIN_VSEL_4SF:
+      return altivec_expand_vec_sel_builtin (CODE_FOR_altivec_vselv4sf, exp,
+                                            target);
+    case ALTIVEC_BUILTIN_VSEL_4SI:
+    case ALTIVEC_BUILTIN_VSEL_4SI_UNS:
+      return altivec_expand_vec_sel_builtin (CODE_FOR_altivec_vselv4si, exp,
+                                            target);
+    case ALTIVEC_BUILTIN_VSEL_8HI:
+    case ALTIVEC_BUILTIN_VSEL_8HI_UNS:
+      return altivec_expand_vec_sel_builtin (CODE_FOR_altivec_vselv8hi, exp,
+                                            target);
+    case ALTIVEC_BUILTIN_VSEL_16QI:
+    case ALTIVEC_BUILTIN_VSEL_16QI_UNS:
+      return altivec_expand_vec_sel_builtin (CODE_FOR_altivec_vselv16qi, exp,
+                                            target);
+
     case ALTIVEC_BUILTIN_DSSALL:
       emit_insn (gen_altivec_dssall ());
       return NULL_RTX;
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index befab53031c..5a5202c455b 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -15566,9 +15566,7 @@ rs6000_emit_vector_cond_expr (rtx dest, rtx op_true, 
rtx op_false,
   machine_mode dest_mode = GET_MODE (dest);
   machine_mode mask_mode = GET_MODE (cc_op0);
   enum rtx_code rcode = GET_CODE (cond);
-  machine_mode cc_mode = CCmode;
   rtx mask;
-  rtx cond2;
   bool invert_move = false;
 
   if (VECTOR_UNIT_NONE_P (dest_mode))
@@ -15608,8 +15606,6 @@ rs6000_emit_vector_cond_expr (rtx dest, rtx op_true, 
rtx op_false,
     case GEU:
     case LTU:
     case LEU:
-      /* Mark unsigned tests with CCUNSmode.  */
-      cc_mode = CCUNSmode;
 
       /* Invert condition to avoid compound test if necessary.  */
       if (rcode == GEU || rcode == LEU)
@@ -15629,6 +15625,9 @@ rs6000_emit_vector_cond_expr (rtx dest, rtx op_true, 
rtx op_false,
   if (!mask)
     return 0;
 
+  if (mask_mode != dest_mode)
+    mask = simplify_gen_subreg (dest_mode, mask, mask_mode, 0);
+
   if (invert_move)
     std::swap (op_true, op_false);
 
@@ -15668,13 +15667,11 @@ rs6000_emit_vector_cond_expr (rtx dest, rtx op_true, 
rtx op_false,
   if (!REG_P (op_false) && !SUBREG_P (op_false))
     op_false = force_reg (dest_mode, op_false);
 
-  cond2 = gen_rtx_fmt_ee (NE, cc_mode, gen_lowpart (dest_mode, mask),
-                         CONST0_RTX (dest_mode));
-  emit_insn (gen_rtx_SET (dest,
-                         gen_rtx_IF_THEN_ELSE (dest_mode,
-                                               cond2,
-                                               op_true,
-                                               op_false)));
+  rtx tmp = gen_rtx_IOR (dest_mode,
+                        gen_rtx_AND (dest_mode, gen_rtx_NOT (dest_mode, mask),
+                                     op_false),
+                        gen_rtx_AND (dest_mode, mask, op_true));
+  emit_insn (gen_rtx_SET (dest, tmp));
   return 1;
 }
 
diff --git a/gcc/config/rs6000/vector.md b/gcc/config/rs6000/vector.md
index 3446b03d40d..85ffcf6a763 100644
--- a/gcc/config/rs6000/vector.md
+++ b/gcc/config/rs6000/vector.md
@@ -862,23 +862,21 @@ (define_insn_and_split "vector_<code><mode>"
 ;; which is in the reverse order that we want
 (define_expand "vector_select_<mode>"
   [(set (match_operand:VEC_L 0 "vlogical_operand")
-       (if_then_else:VEC_L
-        (ne:CC (match_operand:VEC_L 3 "vlogical_operand")
-               (match_dup 4))
-        (match_operand:VEC_L 2 "vlogical_operand")
-        (match_operand:VEC_L 1 "vlogical_operand")))]
-  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)"
-  "operands[4] = CONST0_RTX (<MODE>mode);")
+    (ior:VEC_L
+     (and:VEC_L (not:VEC_L (match_operand:VEC_L 3 "vlogical_operand"))
+      (match_operand:VEC_L 1 "vlogical_operand"))
+     (and:VEC_L (match_dup 3)
+     (match_operand:VEC_L 2 "vlogical_operand"))))]
+  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)")
 
 (define_expand "vector_select_<mode>_uns"
   [(set (match_operand:VEC_L 0 "vlogical_operand")
-       (if_then_else:VEC_L
-        (ne:CCUNS (match_operand:VEC_L 3 "vlogical_operand")
-                  (match_dup 4))
-        (match_operand:VEC_L 2 "vlogical_operand")
-        (match_operand:VEC_L 1 "vlogical_operand")))]
-  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)"
-  "operands[4] = CONST0_RTX (<MODE>mode);")
+    (ior:VEC_L
+     (and:VEC_L (not:VEC_L (match_operand:VEC_L 3 "vlogical_operand"))
+      (match_operand:VEC_L 1 "vlogical_operand"))
+     (and:VEC_L (match_dup 3)
+     (match_operand:VEC_L 2 "vlogical_operand"))))]
+  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)")
 
 ;; Expansions that compare vectors producing a vector result and a predicate,
 ;; setting CR6 to indicate a combined status
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index bcb92be2f5c..f53de7bf34c 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -2105,30 +2105,66 @@ (define_insn "*vsx_ge_<mode>_p"
   [(set_attr "type" "<VStype_simple>")])
 
 ;; Vector select
-(define_insn "*vsx_xxsel<mode>"
+(define_insn "vsx_xxsel<mode>"
   [(set (match_operand:VSX_L 0 "vsx_register_operand" "=<VSr>,?wa")
-       (if_then_else:VSX_L
-        (ne:CC (match_operand:VSX_L 1 "vsx_register_operand" "<VSr>,wa")
-               (match_operand:VSX_L 4 "zero_constant" ""))
-        (match_operand:VSX_L 2 "vsx_register_operand" "<VSr>,wa")
-        (match_operand:VSX_L 3 "vsx_register_operand" "<VSr>,wa")))]
-  "VECTOR_MEM_VSX_P (<MODE>mode)"
-  "xxsel %x0,%x3,%x2,%x1"
+    (ior:VSX_L
+     (and:VSX_L
+      (not:VSX_L (match_operand:VSX_L 3 "vsx_register_operand" "<VSr>,wa"))
+      (match_operand:VSX_L 1 "vsx_register_operand" "<VSr>,wa"))
+     (and:VSX_L
+      (match_operand:VSX_L 2 "vsx_register_operand" "<VSr>,wa")
+      (match_operand:VSX_L 4 "vsx_register_operand" "<VSr>,wa"))))]
+  "VECTOR_MEM_VSX_P (<MODE>mode)
+  && rtx_equal_p (operands[2], operands[3])"
+  "xxsel %x0,%x1,%x4,%x3"
   [(set_attr "type" "vecmove")
    (set_attr "isa" "<VSisa>")])
 
-(define_insn "*vsx_xxsel<mode>_uns"
+(define_insn "vsx_xxsel<mode>2"
   [(set (match_operand:VSX_L 0 "vsx_register_operand" "=<VSr>,?wa")
-       (if_then_else:VSX_L
-        (ne:CCUNS (match_operand:VSX_L 1 "vsx_register_operand" "<VSr>,wa")
-                  (match_operand:VSX_L 4 "zero_constant" ""))
-        (match_operand:VSX_L 2 "vsx_register_operand" "<VSr>,wa")
-        (match_operand:VSX_L 3 "vsx_register_operand" "<VSr>,wa")))]
-  "VECTOR_MEM_VSX_P (<MODE>mode)"
-  "xxsel %x0,%x3,%x2,%x1"
+    (ior:VSX_L
+     (and:VSX_L
+      (not:VSX_L (match_operand:VSX_L 3 "vsx_register_operand" "<VSr>,wa"))
+      (match_operand:VSX_L 1 "vsx_register_operand" "<VSr>,wa"))
+     (and:VSX_L
+      (match_operand:VSX_L 2 "vsx_register_operand" "<VSr>,wa")
+      (match_operand:VSX_L 4 "vsx_register_operand" "<VSr>,wa"))))]
+  "VECTOR_MEM_VSX_P (<MODE>mode)
+  && rtx_equal_p (operands[4], operands[3])"
+  "xxsel %x0,%x1,%x2,%x3"
   [(set_attr "type" "vecmove")
    (set_attr "isa" "<VSisa>")])
 
+(define_insn "vsx_xxsel<mode>3"
+ [(set (match_operand:VSX_L 0 "vsx_register_operand" "=<VSr>,?wa")
+   (ior:VSX_L
+    (and:VSX_L
+     (match_operand:VSX_L 1 "vsx_register_operand" "<VSr>,wa")
+     (match_operand:VSX_L 2 "vsx_register_operand" "<VSr>,wa"))
+    (and:VSX_L
+     (not:VSX_L (match_operand:VSX_L 3 "vsx_register_operand" "<VSr>,wa"))
+     (match_operand:VSX_L 4 "vsx_register_operand" "<VSr>,wa"))))]
+ "VECTOR_MEM_VSX_P (<MODE>mode)
+  && rtx_equal_p (operands[1], operands[3])"
+  "xxsel %x0,%x4,%x2,%x3"
+ [(set_attr "type" "vecmove")
+ (set_attr "isa" "<VSisa>")])
+
+(define_insn "vsx_xxsel<mode>4"
+ [(set (match_operand:VSX_L 0 "vsx_register_operand" "=<VSr>,?wa")
+   (ior:VSX_L
+    (and:VSX_L
+     (match_operand:VSX_L 1 "vsx_register_operand" "<VSr>,wa")
+     (match_operand:VSX_L 2 "vsx_register_operand" "<VSr>,wa"))
+    (and:VSX_L
+     (not:VSX_L (match_operand:VSX_L 3 "vsx_register_operand" "<VSr>,wa"))
+     (match_operand:VSX_L 4 "vsx_register_operand" "<VSr>,wa"))))]
+ "VECTOR_MEM_VSX_P (<MODE>mode)
+ && rtx_equal_p (operands[2], operands[3])"
+ "xxsel %x0,%x4,%x1,%x3"
+ [(set_attr "type" "vecmove")
+ (set_attr "isa" "<VSisa>")])
+
 ;; Copy sign
 (define_insn "vsx_copysign<mode>3"
   [(set (match_operand:VSX_F 0 "vsx_register_operand" "=wa")
diff --git a/gcc/testsuite/gcc.target/powerpc/pr94613.c 
b/gcc/testsuite/gcc.target/powerpc/pr94613.c
new file mode 100644
index 00000000000..06699088f9a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr94613.c
@@ -0,0 +1,38 @@
+/* { dg-do run } */
+/* { dg-require-effective-target vmx_hw } */
+/* { dg-options "-O3 -maltivec" } */
+
+#include <altivec.h>
+
+/* The initial implementation of vec_sel used an IF_THEN_ELSE rtx.
+   This did NOT match what the vsel instruction does.  vsel is a
+   bit-wise operation.  Using IF_THEN_ELSE made the + operation to be
+   simplified away in combine.  A plus operation affects other bits in
+   the same element. Hence per-element simplifications are wrong for
+   vsel.  */
+vector unsigned char __attribute__((noinline))
+foo (vector unsigned char a, vector unsigned char b, vector unsigned char c)
+{
+  return vec_sel (a + b, c, a);
+}
+
+vector unsigned char __attribute__((noinline))
+bar (vector unsigned char a, vector unsigned char b, vector unsigned char c)
+{
+  return vec_sel (a | b, c, a);
+}
+
+int
+main ()
+{
+  vector unsigned char v = (vector unsigned char){ 1 };
+
+  if (foo (v, v, v)[0] != 3)
+      __builtin_abort ();
+
+  if (bar (v, v, v)[0] != 1)
+    __builtin_abort ();
+
+  return 0;
+}
+
-- 
2.25.1

From 74cf1fd298e4923c106deaba3192423d48049559 Mon Sep 17 00:00:00 2001
From: Xionghu Luo <luo...@linux.ibm.com>
Date: Fri, 14 May 2021 01:21:06 -0500
Subject: [PATCH 2/2] rs6000: Fold xxsel to vsel since they have same semantics

gcc/ChangeLog:

        * config/rs6000/altivec.md: Add vsx register constraints.
        * config/rs6000/vsx.md (vsx_xxsel<mode>): Delete.
        (vsx_xxsel<mode>2): Likewise.
        (vsx_xxsel<mode>3): Likewise.
        (vsx_xxsel<mode>4): Likewise.
---
 gcc/config/rs6000/altivec.md | 56 +++++++++++++++++++--------------
 gcc/config/rs6000/vsx.md     | 61 ------------------------------------
 2 files changed, 32 insertions(+), 85 deletions(-)

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index df8f24701d0..6ce3eb9dbc8 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -668,59 +668,67 @@ (define_insn "*altivec_gev4sf"
   [(set_attr "type" "veccmp")])
 
 (define_insn "altivec_vsel<mode>"
-  [(set (match_operand:VM 0 "altivec_register_operand" "=v")
+  [(set (match_operand:VM 0 "altivec_register_operand" "=wa,v")
        (ior:VM
         (and:VM
-         (not:VM (match_operand:VM 3 "altivec_register_operand" "v"))
-         (match_operand:VM 1 "altivec_register_operand" "v"))
+         (not:VM (match_operand:VM 3 "altivec_register_operand" "wa,v"))
+         (match_operand:VM 1 "altivec_register_operand" "wa,v"))
         (and:VM
-         (match_operand:VM 2 "altivec_register_operand" "v")
-         (match_operand:VM 4 "altivec_register_operand" "v"))))]
+         (match_operand:VM 2 "altivec_register_operand" "wa,v")
+         (match_operand:VM 4 "altivec_register_operand" "wa,v"))))]
   "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)
   && rtx_equal_p (operands[2], operands[3])"
-  "vsel %0,%1,%4,%3"
+  "@
+  xxsel %x0,%x1,%x4,%x3
+  vsel %0,%1,%4,%3"
   [(set_attr "type" "vecmove")])
 
 (define_insn "altivec_vsel<mode>2"
-  [(set (match_operand:VM 0 "altivec_register_operand" "=v")
+  [(set (match_operand:VM 0 "altivec_register_operand" "=wa,v")
        (ior:VM
         (and:VM
-         (not:VM (match_operand:VM 3 "altivec_register_operand" "v"))
-         (match_operand:VM 1 "altivec_register_operand" "v"))
+         (not:VM (match_operand:VM 3 "altivec_register_operand" "wa,v"))
+         (match_operand:VM 1 "altivec_register_operand" "wa,v"))
         (and:VM
-         (match_operand:VM 2 "altivec_register_operand" "v")
-         (match_operand:VM 4 "altivec_register_operand" "v"))))]
+         (match_operand:VM 2 "altivec_register_operand" "wa,v")
+         (match_operand:VM 4 "altivec_register_operand" "wa,v"))))]
   "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)
   && rtx_equal_p (operands[4], operands[3])"
-  "vsel %0,%1,%2,%3"
+  "@
+  xxsel %x0,%x1,%x2,%x3
+  vsel %0,%1,%2,%3"
   [(set_attr "type" "vecmove")])
 
 (define_insn "altivec_vsel<mode>3"
-  [(set (match_operand:VM 0 "altivec_register_operand" "=v")
+  [(set (match_operand:VM 0 "altivec_register_operand" "=wa,v")
        (ior:VM
         (and:VM
-         (match_operand:VM 1 "altivec_register_operand" "v")
-         (match_operand:VM 2 "altivec_register_operand" "v"))
+         (match_operand:VM 1 "altivec_register_operand" "wa,v")
+         (match_operand:VM 2 "altivec_register_operand" "wa,v"))
         (and:VM
-         (not:VM (match_operand:VM 3 "altivec_register_operand" "v"))
-         (match_operand:VM 4 "altivec_register_operand" "v"))))]
+         (not:VM (match_operand:VM 3 "altivec_register_operand" "wa,v"))
+         (match_operand:VM 4 "altivec_register_operand" "wa,v"))))]
   "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)
   && rtx_equal_p (operands[1], operands[3])"
-  "vsel %0,%4,%2,%3"
+  "@
+  xxsel %x0,%x4,%x2,%x3
+  vsel %0,%4,%2,%3"
   [(set_attr "type" "vecmove")])
 
 (define_insn "altivec_vsel<mode>4"
-  [(set (match_operand:VM 0 "altivec_register_operand" "=v")
+  [(set (match_operand:VM 0 "altivec_register_operand" "=wa,v")
        (ior:VM
         (and:VM
-         (match_operand:VM 1 "altivec_register_operand" "v")
-         (match_operand:VM 2 "altivec_register_operand" "v"))
+         (match_operand:VM 1 "altivec_register_operand" "wa,v")
+         (match_operand:VM 2 "altivec_register_operand" "wa,v"))
         (and:VM
-         (not:VM (match_operand:VM 3 "altivec_register_operand" "v"))
-         (match_operand:VM 4 "altivec_register_operand" "v"))))]
+         (not:VM (match_operand:VM 3 "altivec_register_operand" "wa,v"))
+         (match_operand:VM 4 "altivec_register_operand" "wa,v"))))]
   "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)
   && rtx_equal_p (operands[2], operands[3])"
-  "vsel %0,%4,%1,%3"
+  "@
+  xxsel %x0,%x4,%x1,%x3
+  vsel %0,%4,%1,%3"
   [(set_attr "type" "vecmove")])
 
 ;; Fused multiply add.
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index f53de7bf34c..55d830d0f20 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -2104,67 +2104,6 @@ (define_insn "*vsx_ge_<mode>_p"
   "xvcmpge<sd>p. %x0,%x1,%x2"
   [(set_attr "type" "<VStype_simple>")])
 
-;; Vector select
-(define_insn "vsx_xxsel<mode>"
-  [(set (match_operand:VSX_L 0 "vsx_register_operand" "=<VSr>,?wa")
-    (ior:VSX_L
-     (and:VSX_L
-      (not:VSX_L (match_operand:VSX_L 3 "vsx_register_operand" "<VSr>,wa"))
-      (match_operand:VSX_L 1 "vsx_register_operand" "<VSr>,wa"))
-     (and:VSX_L
-      (match_operand:VSX_L 2 "vsx_register_operand" "<VSr>,wa")
-      (match_operand:VSX_L 4 "vsx_register_operand" "<VSr>,wa"))))]
-  "VECTOR_MEM_VSX_P (<MODE>mode)
-  && rtx_equal_p (operands[2], operands[3])"
-  "xxsel %x0,%x1,%x4,%x3"
-  [(set_attr "type" "vecmove")
-   (set_attr "isa" "<VSisa>")])
-
-(define_insn "vsx_xxsel<mode>2"
-  [(set (match_operand:VSX_L 0 "vsx_register_operand" "=<VSr>,?wa")
-    (ior:VSX_L
-     (and:VSX_L
-      (not:VSX_L (match_operand:VSX_L 3 "vsx_register_operand" "<VSr>,wa"))
-      (match_operand:VSX_L 1 "vsx_register_operand" "<VSr>,wa"))
-     (and:VSX_L
-      (match_operand:VSX_L 2 "vsx_register_operand" "<VSr>,wa")
-      (match_operand:VSX_L 4 "vsx_register_operand" "<VSr>,wa"))))]
-  "VECTOR_MEM_VSX_P (<MODE>mode)
-  && rtx_equal_p (operands[4], operands[3])"
-  "xxsel %x0,%x1,%x2,%x3"
-  [(set_attr "type" "vecmove")
-   (set_attr "isa" "<VSisa>")])
-
-(define_insn "vsx_xxsel<mode>3"
- [(set (match_operand:VSX_L 0 "vsx_register_operand" "=<VSr>,?wa")
-   (ior:VSX_L
-    (and:VSX_L
-     (match_operand:VSX_L 1 "vsx_register_operand" "<VSr>,wa")
-     (match_operand:VSX_L 2 "vsx_register_operand" "<VSr>,wa"))
-    (and:VSX_L
-     (not:VSX_L (match_operand:VSX_L 3 "vsx_register_operand" "<VSr>,wa"))
-     (match_operand:VSX_L 4 "vsx_register_operand" "<VSr>,wa"))))]
- "VECTOR_MEM_VSX_P (<MODE>mode)
-  && rtx_equal_p (operands[1], operands[3])"
-  "xxsel %x0,%x4,%x2,%x3"
- [(set_attr "type" "vecmove")
- (set_attr "isa" "<VSisa>")])
-
-(define_insn "vsx_xxsel<mode>4"
- [(set (match_operand:VSX_L 0 "vsx_register_operand" "=<VSr>,?wa")
-   (ior:VSX_L
-    (and:VSX_L
-     (match_operand:VSX_L 1 "vsx_register_operand" "<VSr>,wa")
-     (match_operand:VSX_L 2 "vsx_register_operand" "<VSr>,wa"))
-    (and:VSX_L
-     (not:VSX_L (match_operand:VSX_L 3 "vsx_register_operand" "<VSr>,wa"))
-     (match_operand:VSX_L 4 "vsx_register_operand" "<VSr>,wa"))))]
- "VECTOR_MEM_VSX_P (<MODE>mode)
- && rtx_equal_p (operands[2], operands[3])"
- "xxsel %x0,%x4,%x1,%x3"
- [(set_attr "type" "vecmove")
- (set_attr "isa" "<VSisa>")])
-
 ;; Copy sign
 (define_insn "vsx_copysign<mode>3"
   [(set (match_operand:VSX_F 0 "vsx_register_operand" "=wa")
-- 
2.25.1

Reply via email to