Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition

[Jeff Law <law at redhat dot com>]

On 09/26/14 07:57, Felix Yang wrote:
> Hi Jeff,
>
>      Thanks for the suggestions. I updated the patch accordingly.
>
>      1. Both my employer(Huawei) and I have signed the copyright
> assignments with FSF.
>          These assignments are already sent via post two days ago and
> hopefully should reach FSF in one week.
>          Maybe it's OK to commit this patch now?
Not really.  It needs to be accepted by the FSF before we can include 
the work.

>       2. I am not turning member loop_depth of struct equivalence into
> short integer as GCC API such as bb_loop_depth
>           returns a loop's depth as a 32-bit interger.
There's already other places that assume loops don't nest that deep. 
Please go ahead and change it.  And no need to explicitly mark the 
unused bits.  That's just a maintenance nightmare in the long term 
anyway :-)


>       3. I find it's kind of difficult to use the new type and
> interfaces for list walking the init_insns list for this patch.
>          The type of init_insns list is rtx, not rtl_insn_list *. Seems
> we need to change a lot in order to use the new interface.
>          Not clear about the reason why it is not adjusted when we are
> transferring to the new interface.
>          Anyway, I think it's better to have another patch fix that issue. OK?
The right way to go is to add a checked cast when we have some code that 
is using the old interface and other code using the new interface.  It's 
actually a pretty easy change.

The checked casts effectively mark the limits of where we've been able 
to push the RTL typesafety work.  Long term as we push the typesafety 
work further into the compiler many/most of the checked casts will go away.

Unfortunately, that won't work in this case because other code wants to 
store a (const0_rtx) into the insn list.  (const0_rtx) isn't an INSN, so 
the checked cast fails and we get a nice abort/ICE.

Conceptually we just need another marker that is an INSN and we might as 
well just convert the whole file to use the new interface at that point.

Consider the request pulled.

The const0-rtx problem may be why this wasn't converted in the first 
palce.  Or it may simply have been a time problem.  David's done > 250 
patches around RTL typesafety, but he also has other work to be doing ;-)

>       4. This bug is only reproduceable with my local customized GCC
> version. So I don't have a testcase then.
OK.

I'll do a final review when I get notice about the copyright assignment 
from the FSF.

jeff