On Sun, May 01, 2016 at 10:39:48AM -0600, Martin Sebor wrote:
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -7878,50 +7878,101 @@ fold_builtin_unordered_cmp (location_t loc, tree
> fndecl, tree arg0, tree arg1,
>
> static tree
> fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
> - tree arg0, tree arg1, tree arg2)
> + tree arg0, tree arg1, tree arg2 = NULL_TREE)
> {
> enum internal_fn ifn = IFN_LAST;
> - tree type = TREE_TYPE (TREE_TYPE (arg2));
> - tree mem_arg2 = build_fold_indirect_ref_loc (loc, arg2);
> + enum tree_code opcode;
> +
> + /* For the "generic" overloads, the first two arguments can have different
> + types and the last argument determines the target type to use to check
> + for overflow. The names of each of the other overloads determines
> + the target type and so the last argument can be omitted. */
> + tree type = NULL_TREE;
> +
> switch (fcode)
> {
> case BUILT_IN_ADD_OVERFLOW:
> + type = type ? type : TREE_TYPE (TREE_TYPE (arg2));
> case BUILT_IN_SADD_OVERFLOW:
> + type = type ? type : integer_type_node;
> case BUILT_IN_SADDL_OVERFLOW:
> + type = type ? type : long_integer_type_node;
> case BUILT_IN_SADDLL_OVERFLOW:
> + type = type ? type : long_long_integer_type_node;
> case BUILT_IN_UADD_OVERFLOW:
> + type = type ? type : unsigned_type_node;
> case BUILT_IN_UADDL_OVERFLOW:
> + type = type ? type : long_unsigned_type_node;
> case BUILT_IN_UADDLL_OVERFLOW:
> + type = type ? type : long_long_unsigned_type_node;
> ifn = IFN_ADD_OVERFLOW;
> + opcode = PLUS_EXPR;
...
I fail to understand this. You set type to NULL_TREE, and then (not in any
kind of loop, nor any label you could goto to) do:
type = type ? type : xxx;
That is necessarily equal to type = xxx;, isn't it?
> break;
> case BUILT_IN_SUB_OVERFLOW:
> + type = type ? type : TREE_TYPE (TREE_TYPE (arg2));
> case BUILT_IN_SSUB_OVERFLOW:
> + type = type ? type : integer_type_node;
> case BUILT_IN_SSUBL_OVERFLOW:
> + type = type ? type : long_integer_type_node;
> case BUILT_IN_SSUBLL_OVERFLOW:
> + type = type ? type : long_long_integer_type_node;
> case BUILT_IN_USUB_OVERFLOW:
> + type = type ? type : unsigned_type_node;
> case BUILT_IN_USUBL_OVERFLOW:
> + type = type ? type : long_unsigned_type_node;
> case BUILT_IN_USUBLL_OVERFLOW:
> + type = type ? type : long_long_unsigned_type_node;
> ifn = IFN_SUB_OVERFLOW;
> + opcode = MINUS_EXPR;
> break;
> case BUILT_IN_MUL_OVERFLOW:
> + type = type ? type : TREE_TYPE (TREE_TYPE (arg2));
> case BUILT_IN_SMUL_OVERFLOW:
> + type = type ? type : integer_type_node;
> case BUILT_IN_SMULL_OVERFLOW:
> + type = type ? type : long_integer_type_node;
> case BUILT_IN_SMULLL_OVERFLOW:
> + type = type ? type : long_long_integer_type_node;
> case BUILT_IN_UMUL_OVERFLOW:
> + type = type ? type : unsigned_type_node;
> case BUILT_IN_UMULL_OVERFLOW:
> + type = type ? type : long_unsigned_type_node;
> case BUILT_IN_UMULLL_OVERFLOW:
> + type = type ? type : long_long_unsigned_type_node;
> ifn = IFN_MUL_OVERFLOW;
> + opcode = MULT_EXPR;
> break;
> default:
> gcc_unreachable ();
> }
> +
> + /* When the last argument is a NULL pointer and the first two arguments
> + are constant, attempt to fold the built-in call into a constant
> + expression indicating whether or not it detected an overflow but
> + without storing the result. */
> + if ((!arg2 || zerop (arg2))
As I said in another mail, IMNSHO you don't want to change the clang
compatibility builtins, therefore arg2 should never be NULL.
As it is a pointer, shouldn't the above be integer_zerop instead?
Regarding the varargs vs. named args only different diagnostics, if you
think it should change, then IMHO you should submit as independent patch
a change to the diagnostics wording, so that the wording is the same, rather
than changing functions from fixed arguments to varargs (/ typegeneric).
Jakub
