Re: [PATCH] Fix bug in simplify_ternary_operation

[Jeff Law <law@redhat.com>]

On 08/28/2017 12:26 PM, Tom de Vries wrote:
> Hi,
> 
> I think I found a bug in r17465:
> ...
>>        * cse.c (simplify_ternary_operation): Handle more IF_THEN_ELSE
>>        simplifications.
>>
>> diff --git a/gcc/cse.c b/gcc/cse.c
>> index e001597..3c27387 100644
>> --- a/gcc/cse.c
>> +++ b/gcc/cse.c
>> @@ -4713,6 +4713,17 @@ simplify_ternary_operation (code, mode,
>> op0_mode, op0, op1, op2)
> 
> Note: the parameters of simplify_ternary_operation have the following
> meaning:
> ...
> /* Simplify CODE, an operation with result mode MODE and three operands,
>    OP0, OP1, and OP2.  OP0_MODE was the mode of OP0 before it became
>    a constant.  Return 0 if no simplifications is possible.  */
> 
> rtx
> simplify_ternary_operation (code, mode, op0_mode, op0, op1, op2)
>      enum rtx_code code;
>      enum machine_mode mode, op0_mode;
>      rtx op0, op1, op2;
> ...
> 
>>           && rtx_equal_p (XEXP (op0, 1), op1)
>>           && rtx_equal_p (XEXP (op0, 0), op2))
>>         return op2;
>> +      else if (! side_effects_p (op0))
>> +       {
>> +         rtx temp;
>> +         temp = simplify_relational_operation (GET_CODE (op0), op0_mode,
>> +                                               XEXP (op0, 0), XEXP
>> (op0, 1));
> 
> We're handling code == IF_THEN_ELSE here, so op0 is the condition, op1
> is the 'then expr' and op2 is the 'else expr'.
> 
> The parameters of simplify_relational_operation have the following meaning:
> ...
> /* Like simplify_binary_operation except used for relational operators.
>    MODE is the mode of the operands, not that of the result.  If MODE
>    is VOIDmode, both operands must also be VOIDmode and we compare the
>    operands in "infinite precision".
> 
>    If no simplification is possible, this function returns zero.
>    Otherwise, it returns either const_true_rtx or const0_rtx.  */
> 
> rtx
> simplify_relational_operation (code, mode, op0, op1)
>      enum rtx_code code;
>      enum machine_mode mode;
>      rtx op0, op1;
> ...
> 
> The problem in the patch is that we use op0_mode argument for the mode
> parameter. The mode parameter of simplify_relational_operation needs to
> be the mode of the operands of the condition, while op0_mode is the mode
> of the condition.
> 
> Patch below fixes this on current trunk.
> 
> [ I found this by running into an ICE in
> gcc.c-torture/compile/pr28776-2.c for gcn target. I haven't been able to
> reproduce this with an upstream branch yet. ]
> 
> OK for trunk if bootstrap and reg-test for x86_64 succeeds?
So clearly setting cmp_mode to op0_mode is wrong.   But we also have to
make sure that if cmp_mode is VOIDmode that either XEXP (op0, 0) has a
non-void mode or that XEXP (op0, 1) has a non-void mode, otherwise we're
likely to abort down in simplify_const_relational_operation.

ISTM a better fix is to return NULL_RTX if cmp_mode is VOIDmode and both
the sub-operations are VOIDmode as well.

Can you try that and verify that pr28776-2.c continues to work?
jeff