On 23/02/2024 15:13, Richard Biener wrote:
On Fri, 23 Feb 2024, Jakub Jelinek wrote:

On Fri, Feb 23, 2024 at 02:22:19PM +0000, Andrew Stubbs wrote:
On 23/02/2024 13:02, Jakub Jelinek wrote:
On Fri, Feb 23, 2024 at 12:58:53PM +0000, Andrew Stubbs wrote:
This is a follow-up to the previous patch to ensure that integer vector
bit-masks do not have excess bits set. It fixes a bug, observed on
amdgcn, in which the mask could be incorrectly set to zero, resulting in
wrong-code.

The mask was broken when nunits==32. The patched version will probably
be broken for nunits==64, but I don't think any current targets have
masks with more than 64 bits.

OK for mainline?

Andrew

gcc/ChangeLog:

        * expr.cc (store_constructor): Use 64-bit shifts.

No, this isn't 64-bit shift on all hosts.
Use HOST_WIDE_INT_1U instead.

OK, I did wonder if there was a proper way to do it. :)

How about this?

If you change the other two GEN_INT ((1 << nunits) - 1) occurrences in
expr.cc the same way, then LGTM.

There's also two in dojump.cc

This patch should fix all the cases, I think.

I have not observed any further test result changes.

OK?

Andrew
vect: Fix integer overflow calculating mask

The masks and bitvectors were broken when nunits==32 on hosts where int is
32-bit.

gcc/ChangeLog:

        * dojump.cc (do_compare_and_jump): Use full-width integers for shifts.
        * expr.cc (store_constructor): Likewise.
        (do_store_flag): Likewise.

diff --git a/gcc/dojump.cc b/gcc/dojump.cc
index ac744e54cf8..88600cb42d3 100644
--- a/gcc/dojump.cc
+++ b/gcc/dojump.cc
@@ -1318,10 +1318,10 @@ do_compare_and_jump (tree treeop0, tree treeop1, enum 
rtx_code signed_code,
     {
       gcc_assert (code == EQ || code == NE);
       op0 = expand_binop (mode, and_optab, op0,
-                         GEN_INT ((1 << nunits) - 1), NULL_RTX,
+                         GEN_INT ((HOST_WIDE_INT_1U << nunits) - 1), NULL_RTX,
                          true, OPTAB_WIDEN);
       op1 = expand_binop (mode, and_optab, op1,
-                         GEN_INT ((1 << nunits) - 1), NULL_RTX,
+                         GEN_INT ((HOST_WIDE_INT_1U << nunits) - 1), NULL_RTX,
                          true, OPTAB_WIDEN);
     }
 
diff --git a/gcc/expr.cc b/gcc/expr.cc
index 8d34d024c9c..f7d74525c15 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -7879,8 +7879,8 @@ store_constructor (tree exp, rtx target, int cleared, 
poly_int64 size,
            auto nunits = TYPE_VECTOR_SUBPARTS (type).to_constant ();
            if (maybe_ne (GET_MODE_PRECISION (mode), nunits))
              tmp = expand_binop (mode, and_optab, tmp,
-                                 GEN_INT ((1 << nunits) - 1), target,
-                                 true, OPTAB_WIDEN);
+                                 GEN_INT ((HOST_WIDE_INT_1U << nunits) - 1),
+                                 target, true, OPTAB_WIDEN);
            if (tmp != target)
              emit_move_insn (target, tmp);
            break;
@@ -13707,11 +13707,11 @@ do_store_flag (sepops ops, rtx target, machine_mode 
mode)
     {
       gcc_assert (code == EQ || code == NE);
       op0 = expand_binop (mode, and_optab, op0,
-                         GEN_INT ((1 << nunits) - 1), NULL_RTX,
-                         true, OPTAB_WIDEN);
+                         GEN_INT ((HOST_WIDE_INT_1U << nunits) - 1),
+                         NULL_RTX, true, OPTAB_WIDEN);
       op1 = expand_binop (mode, and_optab, op1,
-                         GEN_INT ((1 << nunits) - 1), NULL_RTX,
-                         true, OPTAB_WIDEN);
+                         GEN_INT ((HOST_WIDE_INT_1U << nunits) - 1),
+                         NULL_RTX, true, OPTAB_WIDEN);
     }
 
   if (target == 0)

Reply via email to