On 09/24/14 06:07, Felix Yang wrote:
Hi Jeff,
Thanks for the comments. I updated the patch adding some enhancements.
Bootstrapped on x86_64-suse-linux. Please apply this patch if OK for trunk.
Three points:
1. For multiple-set register, it is not qualified to have a equiv
note once it is marked by no_equiv. The patch is updated with
this consideration.
Correct.
2. For the rtx_insn_list new interface, I noticed that the old
style XEXP accessor macros is still used in function no_equiv.
And I choose to the old style macros with this patch and should
come up with another patch to fix this issue, OK?
I'd rather any new code going in use the updated interfaces. It's
certainly OK to have af followup patch which converts more pre-existing
code to the new interfaces.
3. For the conditions that an insn on the init_insns list which
did not have a note, I reconsider this and find that this can
never happens. So I replaced the check with a gcc assertion.
OK.
Also, I should have asked this earlier, do you have an assignment on
file with the FSF, or does your employer have any kind of blanket
assignment on file with the FSF? These changes are large enough to
require an assignment.
Index: gcc/ira.c
===================================================================
--- gcc/ira.c (revision 215550)
+++ gcc/ira.c (working copy)
@@ -2900,6 +2900,8 @@ struct equivalence
/* Set when an attempt should be made to replace a register
with the associated src_p entry. */
char replace;
+ /* Set if this register has no known equivalence. */
+ char no_equiv;
};
As a follow-up, can you turn is_arg_equivalence, replace and no_equiv
into boolean bitfields and turn loop_depth into a short (to match
assumptions elsewhere in GCC).
The point is to get better packing of these objects and ultimately use
less memory.
+
+ /* Check if it is possible that this multiple-set register has
+ a known equivalence. */
+ if (reg_equiv[regno].no_equiv)
+ continue;
This comment is a bit confusing. Please consider something like
/* If we have already processed this pseudo and determined it
can not have an equivalence, then honor that decision. */
Do you have a testcase we can add to the regression suite? If at all
possible please include one. An execution test would be best, but you
could also scan the RTL for bogus REG_EQUIV notes.
Please update to use the new type and interfaces for list walking the
init_insns list.
Finally, you need to verify your patch will bootstrap and not cause any
regressions in the testsuite. If you're unsure how to do that, let me know.
I think we'll be ready to go once those tasks are complete.
jeff
