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

[Felix Yang <fei dot yang0953 at gmail dot com>]

Thanks for the explaination.
I have changed the loop_depth into a short interger hoping that we can
save some memory :-)
Attached please find the updated patch. Bootstrapped and reg-tested on
x86_64-suse-linux.
Please do a final revew once the assignment is ready.

As for the new list walking interface, I choose the function
"no_equiv" and tried the "checked cast" way.
The bad news is that GCC failed to bootstrap with the following change:

Index: ira.c
===================================================================
--- ira.c (revision 215536)
+++ ira.c (working copy)
@@ -3242,12 +3242,12 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
   void *data ATTRIBUTE_UNUSED)
 {
   int regno;
-  rtx list;
+  rtx_insn_list *list;

   if (!REG_P (reg))
     return;
   regno = REGNO (reg);
-  list = reg_equiv[regno].init_insns;
+  list = as_a <rtx_insn_list *> (reg_equiv[regno].init_insns);
   if (list == const0_rtx)
     return;
   reg_equiv[regno].init_insns = const0_rtx;
@@ -3258,9 +3258,9 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
     return;
   ira_reg_equiv[regno].defined_p = false;
   ira_reg_equiv[regno].init_insns = NULL;
-  for (; list; list =  XEXP (list, 1))
+  for (; list; list = list->next ())
     {
-      rtx insn = XEXP (list, 0);
+      rtx_insn *insn = list->insn ();
       remove_note (insn, find_reg_note (insn, REG_EQUIV, NULL_RTX));
     }
 }

Error message:
.... ...
checking for suffix of object files... configure: error: in
`/home/yangfei/gcc-devel/build/x86_64-unknown-linux-gnu/libgcc':
configure: error: cannot compute suffix of object files: cannot compile
See `config.log' for more details.
make[2]: *** [configure-stage1-target-libgcc] Error 1
make[2]: Leaving directory `/home/yangfei/gcc-devel/build'
make[1]: *** [stage1-bubble] Error 2
make[1]: Leaving directory `/home/yangfei/gcc-devel/build'
make: *** [all] Error 2

I think the code change is OK. Anything I missed?

Cheers,
Felix


On Sat, Sep 27, 2014 at 5:03 AM, Jeff Law <law@redhat.com> wrote:
> 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
>

Attachment: ira-update-equiv-regs-r3.diff
Description: Text document