On Tue, 14 May 2013, Richard Biener wrote:

On Tue, May 14, 2013 at 1:23 PM, Marc Glisse <marc.gli...@inria.fr> wrote:
Hello,

in an earlier patch I apparently introduced a platform dependent signed /
unsigned comparison, so here is a fix. I am taking the chance to fix the
host_integerp second argument nearby: we want non-negative integers.

Passes bootstrap+testsuite on x86_64-linux-gnu and apparently bootstrap on
m68k.

2013-05-13  Marc Glisse  <marc.gli...@inria.fr>

        PR bootstrap/57266
        * fold-const.c (fold_binary_loc) <shift>: Use an unsigned
        variable for the shift amount. Check that we shift by non-negative
        amounts.

--
Marc Glisse
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c    (revision 198853)
+++ gcc/fold-const.c    (working copy)
@@ -12416,27 +12416,27 @@ fold_binary_loc (location_t loc,
        return fold_build2_loc (loc, code, type, op0, tem);

       /* Since negative shift count is not well-defined,
         don't try to compute it in the compiler.  */
       if (TREE_CODE (arg1) == INTEGER_CST && tree_int_cst_sgn (arg1) < 0)
        return NULL_TREE;

       prec = element_precision (type);

       /* Turn (a OP c1) OP c2 into a OP (c1+c2).  */
-      if (TREE_CODE (op0) == code && host_integerp (arg1, false)
+      if (TREE_CODE (op0) == code && host_integerp (arg1, true)
          && TREE_INT_CST_LOW (arg1) < prec
-         && host_integerp (TREE_OPERAND (arg0, 1), false)
+         && host_integerp (TREE_OPERAND (arg0, 1), true)
          && TREE_INT_CST_LOW (TREE_OPERAND (arg0, 1)) < prec)

That looks good, but ...

        {
-         HOST_WIDE_INT low = (TREE_INT_CST_LOW (TREE_OPERAND (arg0, 1))
-                              + TREE_INT_CST_LOW (arg1));
+         unsigned int low = (TREE_INT_CST_LOW (TREE_OPERAND (arg0, 1))
+                             + TREE_INT_CST_LOW (arg1));

that's wrong.  TREE_INT_CST_LOW doesn't fit in an unsigned int.
unsigned HOST_WIDE_INT would work though.

The checks above show that both TREE_INT_CST_LOW are smaller than prec (for which I already used an unsigned int, but an unsigned char might have been enough until Kenneth introduces int512_t). Do you still want the change?

Ok with that change.

Thanks,
Richard.

          /* Deal with a OP (c1 + c2) being undefined but (a OP c1) OP c2
             being well defined.  */
          if (low >= prec)
            {
              if (code == LROTATE_EXPR || code == RROTATE_EXPR)
                low = low % prec;
              else if (TYPE_UNSIGNED (type) || code == LSHIFT_EXPR)
                return omit_one_operand_loc (loc, type, build_zero_cst
(type),
                                         TREE_OPERAND (arg0, 0));

--
Marc Glisse

Reply via email to