This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: PR37363: PR36090 and PR36182 all over again
Paolo Bonzini <bonzini@gnu.org> writes:
>> I'm not sure about this bit. Couldn't [snip cse.c code]
>> simply be replaced by:
>>
>> /* We can't simplify extension ops unless we know the
>> original mode. */
>> if ((code == ZERO_EXTEND || code == SIGN_EXTEND)
>> && mode_arg0 == VOIDmode)
>> break;
>>
>> new = simplify_unary_operation (code, mode,
>> const_arg0 ? const_arg0 : folded_arg0,
>> mode_arg0);
>> ?
>>
>> (Sorry if I'm repeating earlier discussion here.)
>
> I think so -- I was just trying to resemble the existing code as much as
> possible (stage3),
Understood. FWIW, if we have to apply this to a branch at any point,
I'd be tempted to leave cse.c unchanged.
> What do you thing about the simplify-rtx.c part instead?
Hmm. Well...
> Index: simplify-rtx.c
> ===================================================================
> --- simplify-rtx.c (revision 140055)
> +++ simplify-rtx.c (working copy)
> @@ -3625,7 +3625,7 @@ simplify_plus_minus (enum rtx_code code,
> tem = simplify_binary_operation (ncode, mode, tem_lhs, tem_rhs);
>
> if (tem && !CONSTANT_P (tem))
> - tem = gen_rtx_CONST (GET_MODE (tem), tem);
> + tem = plus_constant (tem, 0);
> }
> else
> tem = simplify_binary_operation (ncode, mode, lhs, rhs);
...I'm not sure about this one. I think the same principle applies:
if plus_constant _knows_ that something can be wrapped in a CONST,
simplify_binary_operation should have given us the CONST to begin with.
Also, the only cases that plus_constant can handle are CONST,
SYMBOL_REF and LABEL_REF, all of which satisfy CONSTANT_P.
So the new form ought to be dead on two counts.
This code needs to handle cases that plus_constant doesn't know about.
E.g. some UNSPECs are inherently constants, and the target can wrap
those UNSPECs in a CONST. If we're given:
(const (plus (unspec [...]) (const_int FOO)))
and we strip the CONST (as we do here), we must be sure to add it
back after meddling with the innards.
It isn't immediately obvious to me why we need to strip CONSTs in
the recursive call. I suppose it must have something to do with
infinite recursion, but really, I think we should try to pass
"canonical" rtl to simplify_binary_operation if possible.
It feels like we might be breaking the infinite recursion in
a less-than-ideal part of the code.
Sorry for not offering a constructive alternative. I'd need
to do some archaeology first.
Richard