"Robin Dapp" <[email protected]> writes:
>> It would be nice if this kind of patch submission came with more
>> analysis.
>> As it stands, it feels like I have to look at the PR independently and see
>> whether I come to the same conclusion.
>
> Apologies, it was absolutely not my intention to cause you additional
> work :/ On the contrary, I'll usually think twice before CC'ing you in
> order not to bother you more often than "necessary".
I don't mind being cc:ed on things :) But yeah, it was just that some
more explanation would have helped in this case.
> Admittedly, I didn't look much further than the concatn, then verified
> that return NULL does nothing harmful and figured that's all we need.
> (And it isn't harmful, right? It is no comprehensive fix like yours, of
> course.)
>
> I gave your patch a spin and found that it still needs:
>
> diff --git a/gcc/lower-subreg.cc b/gcc/lower-subreg.cc
> index b304e25dce0..066d32bf56b 100644
> --- a/gcc/lower-subreg.cc
> +++ b/gcc/lower-subreg.cc
> @@ -1270,7 +1270,7 @@ resolve_debug (rtx_insn *insn)
> if (x)
> *loc = x;
> }
> - if (resolve_reg_p (x))
> + if (x && resolve_reg_p (x))
> {
> *loc = copy_rtx (x);
> DECOMPOSED_REGISTER_P (*loc) = false;
>
> when simplify_subreg_concatn returns NULL we'll dereference it.
>
> I'm re-testing with that change.
Oops, yeah. But perhaps it would be better to make the "if" an
"else if" instead, since the subreg and reg cases are mutually exclusive.
Thanks,
Richard
