Message ID | 5420CC52.8030707@redhat.com |
---|---|
State | New |
Headers | show |
On Tue, Sep 23, 2014 at 3:26 AM, Vladimir Makarov <vmakarov@redhat.com> wrote: > The previous patch to solve PR61360 fixed the problem in IRA (it was > easier for me to do as I know the code well) > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61360 > > Although imo it was an ok fix, Richard expressed concerns with the patch > and the practice to have different enable attribute values depending on the > current pass. > > I don't understand why "x,m" alternative is better to "x,r" and "x,r" > should be disabled. Even if the path from general regs to sse regs is slow > (usually such slow path is implemented internally by micro-architecture > through cache). "x,r" alternative results in only smaller insns (including > number of insns) with probably the same time for the movement. So "x,r" > should be at least no slower, insn cache should have more locality, and less > overhead for decoding/translating insns. > > Here I propose another solution avoiding to have different enable > attribute values. > > The patch was successfully bootstrapped on x86/x86-64 and tested with and > without -march=amdfam10 (actually the patch results in 2 less failures when > -march=amdfam10 were used). > > Uros, is i386.md change ok for the trunk? I don't think so. This would be a regression, since 4.8 (and later versions until Richard's patch) were able to handle this functionality just fine. Please also note that there are a couple of other patterns with the same problem, that is using ("nonimmediate_operand" "m") constraint. Please see PR 60704, comment 7 [1]. If LRA is able to fixup ("nonimmediate_operand" "m") to a memory from a register, then other RTL infrastructure should also be updated for this functionality. IMO, recog should be fixed/enhanced, so pseudo registers will also satisfy ("nonimmediate_operand" "m") constraint before LRA. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60704#c7 Uros.
Index: config/i386/i386.md =================================================================== --- config/i386/i386.md (revision 215337) +++ config/i386/i386.md (working copy) @@ -4795,14 +4795,6 @@ (symbol_ref "TARGET_MIX_SSE_I387 && X87_ENABLE_FLOAT (<MODEF:MODE>mode, <SWI48:MODE>mode)") - (eq_attr "alternative" "1") - /* ??? For sched1 we need constrain_operands to be able to - select an alternative. Leave this enabled before RA. */ - (symbol_ref "TARGET_INTER_UNIT_CONVERSIONS - || optimize_function_for_size_p (cfun) - || !(reload_completed - || reload_in_progress - || lra_in_progress)") ] (symbol_ref "true"))) ]) Index: lra.c =================================================================== --- lra.c (revision 215358) +++ lra.c (working copy) @@ -2135,11 +2135,6 @@ lra (FILE *f) lra_in_progress = 1; - /* The enable attributes can change their values as LRA starts - although it is a bad practice. To prevent reuse of the outdated - values, clear them. */ - recog_init (); - lra_live_range_iter = lra_coalesce_iter = 0; lra_constraint_iter = lra_constraint_iter_after_spill = 0; lra_inheritance_iter = lra_undo_inheritance_iter = 0;