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] fix pr39867, wrong folding to MAX_EXPR


On Fri, Apr 24, 2009 at 8:40 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> This is a fix for PR39867, a MAX operation created from a COND_EXPR
> with the wrong signedness. ?The MAX operation must always be created
> with the operand type of the comparison, and then possibly casted
> to the result type of the COND_EXPR.
>
> Bootstrapped/regtested i686-pc-linux-gnu, ok for trunk and 4.4?

Doesn't the same problem exist with folding to min?

Ok.

Thanks,
Richard.

> Paolo
>
> 2009-04-24 ?Paolo Bonzini ?<bonzini@gnu.org>
>
> ? ? ? ?* fold-const.c (fold_cond_expr_with_comparison): When folding
> ? ? ? ?> and >= to MAX, make sure the MAX uses the same type as the
> ? ? ? ?comparison's operands.
>
> 2009-04-24 ?Paolo Bonzini ?<bonzini@gnu.org>
>
> ? ? ? ?* gcc.dg/pr39867.c: New.
>
> Index: fold-const.c
> ===================================================================
> --- fold-const.c ? ? ? ?(revision 146581)
> +++ fold-const.c ? ? ? ?(working copy)
> @@ -5337,31 +5337,34 @@ fold_cond_expr_with_comparison (tree typ
> ? ? ? ?break;
>
> ? ? ? case GT_EXPR:
> - ? ? ? /* If C1 is C2 - 1, this is max(A, C2). ?*/
> + ? ? ? /* If C1 is C2 - 1, this is max(A, C2), but use ARG00's type for
> + ? ? ? ? ?MAX_EXPR, to preserve the signedness of the comparison. ?*/
> ? ? ? ?if (! operand_equal_p (arg2, TYPE_MIN_VALUE (type),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? OEP_ONLY_CONST)
> ? ? ? ? ? ?&& operand_equal_p (arg01,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const_binop (MINUS_EXPR, arg2,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? build_int_cst (type, 1), 0),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?OEP_ONLY_CONST))
> - ? ? ? ? return pedantic_non_lvalue (fold_build2 (MAX_EXPR,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?type,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?fold_convert (type, arg1),
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?arg2));
> + ? ? ? ? return pedantic_non_lvalue (fold_convert (type,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? fold_build2 (MAX_EXPR, TREE_TYPE (arg00),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?arg00,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?fold_convert (TREE_TYPE (arg00),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?arg2))));
> ? ? ? ?break;
>
> ? ? ? case GE_EXPR:
> - ? ? ? /* If C1 is C2 + 1, this is max(A, C2). ?*/
> + ? ? ? /* If C1 is C2 + 1, this is max(A, C2), with the same care as above. ?*/
> ? ? ? ?if (! operand_equal_p (arg2, TYPE_MAX_VALUE (type),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? OEP_ONLY_CONST)
> ? ? ? ? ? ?&& operand_equal_p (arg01,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const_binop (PLUS_EXPR, arg2,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? build_int_cst (type, 1), 0),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?OEP_ONLY_CONST))
> - ? ? ? ? return pedantic_non_lvalue (fold_build2 (MAX_EXPR,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?type,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?fold_convert (type, arg1),
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?arg2));
> + ? ? ? ? return pedantic_non_lvalue (fold_convert (type,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? fold_build2 (MAX_EXPR, TREE_TYPE (arg00),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?arg00,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?fold_convert (TREE_TYPE (arg00),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?arg2))));
> ? ? ? ?break;
> ? ? ? case NE_EXPR:
> ? ? ? ?break;
>
>
>
>
> /* { dg-do link } */
> /* { dg-options "-O2" } */
>
> int main (void)
> {
> ?int exp = -1;
> ?/* Wrong folding of the LHS to an unsigned MAX leads to 4294967295 >
> ?* 2. ?*/
> ?if (("%u\n", exp < 2 ? 2U : (unsigned int) exp) > 2)
> ? ?link_error ();
> ?return 0;
> }
>
>


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