On 06/03/2016 09:32 AM, Jakub Jelinek wrote:
> On Fri, Jun 03, 2016 at 09:29:48AM -0600, Martin Sebor wrote:
>>>> +	{
>>>> +	  tree type = TREE_TYPE (TREE_TYPE (t));
>>>> +	  tree vflow = arith_overflowed_p (opcode, type, arg0, arg1)
>>>> +	               ? integer_one_node : integer_zero_node;
>>>
>>> This looks incorrect, the return type is TREE_TYPE (t), some complex integer
>>> type, therefore vflow needs to be
>>>        tree vflow = build_int_cst (TREE_TYPE (TREE_TYPE (t)),
>>> 				  arith_overflowed_p (opcode, type, arg0, arg1)
>>> 				  ? 1 : 0);
>>> no?
>>
>> I guess it didn't think it mattered since the complex type specifies
>> the types of the two members.  I don't mind changing it if it does
>
> Sure, it does.  But if there are any differences between the lhs and rhs
> type (or e.g. in COMPLEX_EXPR args etc. in GENERIC), then it is invalid IL,
> or for GIMPLE if the two types aren't compatible according to the GIMPLE
> rules (useless conversion).

I see.  I've made the change in the latest update to the patch
but I wasn't able to create a test case to verify it.  Maybe
that's because this is constexpr the COMPLEX_EXPR doesn't make
it far enough to trigger a problem.  If there is a way to test
it I'd appreciate a suggestion for how (otherwise, if not caught
in a code review like in this case, it would be a ticking time
bomb).

It also occurred to me that a more robust solution might be to
change build_complex to either enforce as a precondition that
the members have a type that matches the complex type.  I've
taken the liberty of making this change as part of this patch.
(It seems that an even better solution would be to have
build_complex convert the arguments to the expected type
so that callers don't have to worry about it.)

Martin