This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Canonicalize comparisons, fix PR26899


Hi Richard,

On Wed, 25 Oct 2006, Richard Guenther wrote:
> +       /* CST <= B  ->  CST-1 < B.  */
> +       /* A - CST < B  ->  A - CST-1 <= B.  */
> ...
> +   /* Now build the reduced constant.  */
> +   *arg0p = int_const_binop (sgn0 == -1 ? PLUS_EXPR : MINUS_EXPR, cst0,
> + 			    build_int_cst (TREE_TYPE (cst0), 1), 0);
> +   if (code0 != INTEGER_CST)
> +     *arg0p = fold_build2 (code0, TREE_TYPE (arg0),
> + 			  TREE_OPERAND (arg0, 0), *arg0p);

I think there's an potential problem here that you're not checking
whether the CST-1 operation overflowed.  For example, "INT_MIN <= x"
would be transformed into "INT_MAX' < x".  Of course, a more realistic
example, is "A - INT_MIN < B"...  I think adding a simple
TREE_OVERFLOW test after the call to int_const_binop should be
sufficient.

It might also be possible to tidy up the maybe_canonicalize_comparison
<-> maybe_canonicalize_comparison_1 API a little bit.  The large use
of pointer indirection in maybe_canonicalize_comparison_1 looks a bit
ugly.  One approach might be to pass in a swapped flag and both arguments
form maybe_canonicalize_comparison, and then return a tree if anything
changed.

maybe_canonicalize_comparison then becomes:

   tree t;
   ...
   t = maybe_canonicalize_comparison_1 (code, arg0, arg1, false);
   if (t)
     return t;
   code = swap_tree_comparison_code (code);
   return maybe_canonicalize_comparison_1 (code, arg1, arg0, true);
}

and allow this to refactor the calls to fold_build2 into the
maybe_canonicalize_comparison_1, thereby avoiding all the pointer
dereferencing.  With luck the resulting code is shorter, and
the returning a non-NULL tree if things change fits better with
the rest of fold-const.c's APIs.


I do like this improvement/canonicalization, and your new test
case demonstrates the potential benefits.

Roger
--


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]