LRA has been merged into trunk.

LRA has been merged into trunk.

[Vladimir Makarov]

[-- Attachment #1: Type: text/plain, Size: 7886 bytes --]

   Hi, I was going to merge LRA into trunk last Sunday.  It did not 
happen.  LRA was actively changed last 4 weeks by implementing 
reviewer's proposals which resulted in a lot of new LRA regressions on 
GCC testsuite in comparison with reload.  Finally, they were fixed and 
everything looks ok to me.

   So I've committed the patch into trunk as rev. 192719. The final 
patch is in the attachment.

2012-10-23  Vladimir Makarov  <vmakarov@redhat.com>

     * dbxout.c (dbxout_symbol_location): Pass new argument to
     alter_subreg.
     * dwarf2out.c: Include ira.h and lra.h.
     (based_loc_descr, compute_frame_pointer_to_fb_displacement): Use
     lra_eliminate_regs for LRA instead of eliminate_regs.
     * expr.c (emit_move_insn_1): Pass an additional argument to
     emit_move_via_integer.  Use emit_move_via_integer for LRA only if
     the insn is recognized.
     * emit-rtl.c (gen_rtx_REG): Add lra_in_progress.
     (validate_subreg): Don't check offset for LRA and floating point
     modes.
     * final.c (final_scan_insn, cleanup_subreg_operands): Pass new
     argument to alter_subreg.
     (walk_alter_subreg, output_operand): Ditto.
     (alter_subreg): Add new argument.
     * gcse.c (calculate_bb_reg_pressure): Add parameter to
     ira_setup_eliminable_regset call.
     * ira.c: Include lra.h.
     (ira_init_once, ira_init, ira_finish_once): Call lra_start_once,
     lra_init, lra_finish_once in anyway.
     (ira_setup_eliminable_regset): Add parameter.  Remove need_fp.
     Call lra_init_elimination and mark HARD_FRAME_POINTER_REGNUM as
     living forever if frame_pointer_needed.
     (setup_reg_class_relations): Set up ira_reg_class_subset.
     (ira_reg_equiv_invariant_p, ira_reg_equiv_const): Remove.
     (find_reg_equiv_invariant_const): Ditto.
     (setup_reg_renumber): Use ira_equiv_no_lvalue_p instead of
     ira_reg_equiv_invariant_p.  Skip caps for LRA.
     (setup_reg_equiv_init, ira_update_equiv_info_by_shuffle_insn): New
     functions.
     (ira_reg_equiv_len, ira_reg_equiv): New externals.
     (ira_reg_equiv): New.
     (ira_expand_reg_equiv, init_reg_equiv, finish_reg_equiv): New
     functions.
     (no_equiv, update_equiv_regs): Use ira_reg_equiv instead of
     reg_equiv_init.
     (setup_reg_equiv): New function.
     (ira_use_lra_p): New global.
     (ira): Set up lra_simple_p and ira_conflicts_p.  Set up and
     restore flag_caller_saves and flag_ira_region.  Move
     initialization of ira_obstack and ira_bitmap_obstack upper.  Call
     init_reg_equiv, setup_reg_equiv, and setup_reg_equiv_init instead
     of initialization of ira_reg_equiv_len, ira_reg_equiv_invariant_p,
     and ira_reg_equiv_const.  Call ira_setup_eliminable_regset with a
     new argument.  Don't flatten IRA IRA for LRA.  Don't reassign
     conflict allocnos for LRA. Call finish_reg_equiv.
         (do_reload): Prepare code for LRA call.  Call LRA.
     * ira.h (ira_use_lra_p): New external.
     (struct target_ira): Add members x_ira_class_subset_p
     x_ira_reg_class_subset, and x_ira_reg_classes_intersect_p.
     (ira_class_subset_p, ira_reg_class_subset): New macros.
     (ira_reg_classes_intersect_p): New macro.
     (struct ira_reg_equiv): New.
     (ira_setup_eliminable_regset): Add an argument.
     (ira_expand_reg_equiv, ira_update_equiv_info_by_shuffle_insn): New
     prototypes.
     * ira-color.c (color_pass, move_spill_restore, coalesce_allocnos):
     Use ira_equiv_no_lvalue_p.
     (coalesce_spill_slots, ira_sort_regnos_for_alter_reg): Ditto.
     * ira-emit.c (ira_create_new_reg): Call ira_expand_reg_equiv.
     (generate_edge_moves, change_loop) Use ira_equiv_no_lvalue_p.
     (emit_move_list): Simplify code.  Call
     ira_update_equiv_info_by_shuffle_insn.  Use ira_reg_equiv instead
     of ira_reg_equiv_invariant_p and ira_reg_equiv_const. Change
     assert.
     * ira-int.h (struct target_ira_int): Remove x_ira_class_subset_p
     and x_ira_reg_classes_intersect_p.
     (ira_class_subset_p, ira_reg_classes_intersect_p): Remove.
     (ira_reg_equiv_len, ira_reg_equiv_invariant_p): Ditto.
     (ira_reg_equiv_const): Ditto.
     (ira_equiv_no_lvalue_p): New function.
     * jump.c (true_regnum): Always use hard_regno for subreg_get_info
     when lra is in progress.
     * haifa-sched.c (sched_init): Pass new argument to
     ira_setup_eliminable_regset.
     * loop-invariant.c (calculate_loop_reg_pressure): Pass new
     argument to ira_setup_eliminable_regset.
     * lra.h: New.
     * lra-int.h: Ditto.
     * lra.c: Ditto.
     * lra-assigns.c: Ditto.
     * lra-constraints.c: Ditto.
     * lra-coalesce.c: Ditto.
     * lra-eliminations.c: Ditto.
     * lra-lives.c: Ditto.
     * lra-spills.c: Ditto.
     * Makefile.in (LRA_INT_H): New.
     (OBJS): Add lra.o, lra-assigns.o, lra-coalesce.o,
     lra-constraints.o, lra-eliminations.o, lra-lives.o, and
     lra-spills.o.
     (dwarf2out.o): Add dependence on ira.h and lra.h.
     (ira.o): Add dependence on lra.h.
     (lra.o, lra-assigns.o, lra-coalesce.o, lra-constraints.o): New
     entries.
     (lra-eliminations.o, lra-lives.o, lra-spills.o): Ditto.
     * output.h (alter_subreg): Add new argument.
     * rtlanal.c (simplify_subreg_regno): Permit mode changes for LRA.
     Permit ARG_POINTER_REGNUM and STACK_POINTER_REGNUM for LRA.
     * recog.c (general_operand, register_operand): Accept paradoxical
     FLOAT_MODE subregs for LRA.
     (scratch_operand): Accept pseudos for LRA.
     * rtl.h (lra_in_progress): New external.
     (debug_bb_n_slim, debug_bb_slim, print_value_slim): New
     prototypes.
     (debug_rtl_slim, debug_insn_slim): Ditto.
     * sdbout.c (sdbout_symbol): Pass new argument to alter_subreg.
     * sched-vis.c (print_value_slim): New.
     * target.def (lra_p): New hook.
     (register_priority): Ditto.
     (different_addr_displacement_p): Ditto.
     (spill_class): Ditto.
     * target-globals.h (this_target_lra_int): New external.
     (target_globals): New member lra_int.
     (restore_target_globals): Restore this_target_lra_int.
     * target-globals.c: Include lra-int.h.
     (default_target_globals): Add &default_target_lra_int.
     * targhooks.c (default_lra_p): New function.
     (default_register_priority): Ditto.
     (default_different_addr_displacement_p): Ditto.
     * targhooks.h (default_lra_p): Declare.
     (default_register_priority): Ditto.
     (default_different_addr_displacement_p): Ditto.
     * timevar.def (TV_LRA, TV_LRA_ELIMINATE, TV_LRA_INHERITANCE): New.
     (TV_LRA_CREATE_LIVE_RANGES, TV_LRA_ASSIGN, TV_LRA_COALESCE): New.
     * config/arm/arm.c (load_multiple_sequence): Pass new argument to^[OB
     alter_subreg.
     (store_multiple_sequence): Ditto.
     * config/i386/i386.h (enum ix86_tune_indices): Add
     X86_TUNE_GENERAL_REGS_SSE_SPILL.
     (TARGET_GENERAL_REGS_SSE_SPILL): New macro.
     * config/i386/i386.c (initial_ix86_tune_features): Set up
     X86_TUNE_GENERAL_REGS_SSE_SPILL for m_COREI7 and m_CORE2I7.
     (ix86_lra_p, ix86_register_priority): New functions.
     (ix86_secondary_reload): Add NON_Q_REGS, SIREG, DIREG.
     (inline_secondary_memory_needed): Change assert.
     (ix86_spill_class): New function.
     (TARGET_LRA_P, TARGET_REGISTER_BANK, TARGET_SPILL_CLASS): New
     macros.
     * config/m68k/m68k.c (emit_move_sequence): Pass new argument to
     alter_subreg.
     * config/m32r/m32r.c (gen_split_move_double): Ditto.
     * config/pa/pa.c (pa_emit_move_sequence): Ditto.
     * config/sh/sh.md: Ditto.
     * config/v850/v850.c (v850_reorg): Ditto.
     * config/xtensa/xtensa.c (fixup_subreg_mem): Ditto.
     * doc/md.texi: Add new interpretation of hint * for LRA.
     * doc/passes.texi: Describe LRA pass.
     * doc/tm.texi.in: Add TARGET_LRA_P, TARGET_REGISTER_PRIORITY,
     TARGET_DIFFERENT_ADDR_DISPLACEMENT_P, and TARGET_SPILL_CLASS.
     * doc/tm.texi: Update.



[-- Attachment #2: lra2.patch.gz --]
[-- Type: x-zip/application, Size: 125551 bytes --]

Re: LRA has been merged into trunk.

[David Miller]

From: Vladimir Makarov <vmakarov@redhat.com>
Date: Tue, 23 Oct 2012 11:46:34 -0400

>   Hi, I was going to merge LRA into trunk last Sunday.  It did not
>   happen.  LRA was actively changed last 4 weeks by implementing
>   reviewer's proposals which resulted in a lot of new LRA regressions on
>   GCC testsuite in comparison with reload.  Finally, they were fixed and
>   everything looks ok to me.
> 
>   So I've committed the patch into trunk as rev. 192719. The final patch
>   is in the attachment.

Thanks for doing this work Vlad.

Just as a heads up I started playing with LRA on Sparc.

Re: LRA has been merged into trunk.

[Vladimir Makarov]

On 12-10-23 5:28 PM, David Miller wrote:
> From: Vladimir Makarov <vmakarov@redhat.com>
> Date: Tue, 23 Oct 2012 11:46:34 -0400
>
>>    Hi, I was going to merge LRA into trunk last Sunday.  It did not
>>    happen.  LRA was actively changed last 4 weeks by implementing
>>    reviewer's proposals which resulted in a lot of new LRA regressions on
>>    GCC testsuite in comparison with reload.  Finally, they were fixed and
>>    everything looks ok to me.
>>
>>    So I've committed the patch into trunk as rev. 192719. The final patch
>>    is in the attachment.
> Thanks for doing this work Vlad.
>
> Just as a heads up I started playing with LRA on Sparc.
Thanks, David.  I am not sure that anything except x86/x86-64 will work 
now on the branch.  There were too many changes on the branch and I 
tested only x86/x86-64.  I'll start testing the rest of targets on the 
branch next week when LRA is settled on trunk.

Re: LRA has been merged into trunk.

[David Miller]

From: Vladimir Makarov <vmakarov@redhat.com>
Date: Tue, 23 Oct 2012 19:04:03 -0400

> I am not sure that anything except x86/x86-64 will work now on the
> branch.  There were too many changes on the branch and I tested only
> x86/x86-64.  I'll start testing the rest of targets on the branch
> next week when LRA is settled on trunk.

I anticipated problems, I wanted to work on solving these puzzles
as it's quite fun :-)

The first issue sparc runs into is that it does not define it's
extra constraints properly.  In particular 'T' and 'W' must use
define_memory_constraint.

Otherwise the EXTRA_MEMORY_CONSTRAINT logic in process_alt_operands()
does not trigger and we therefore cannot even load a constant into
floating point registers properly.

Re: LRA has been merged into trunk.

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 23 Oct 2012 21:44:05 -0400 (EDT)

> The first issue sparc runs into is that it does not define it's
> extra constraints properly.  In particular 'T' and 'W' must use
> define_memory_constraint.
> 
> Otherwise the EXTRA_MEMORY_CONSTRAINT logic in process_alt_operands()
> does not trigger and we therefore cannot even load a constant into
> floating point registers properly.

Ok, the next problem we hit will be a little bit more difficult
to solve.

Sparc accepts addresses of the form:

(plus:DI (lo_sum:DI (reg/f:DI 282)
        (symbol_ref:DI ("__mf_opts") <var_decl 0xf78d74a0 __mf_opts>))
    (const_int 40 [0x28]))

These make use of Sparc's offsetable %lo() relocations.

But LRA is unable to cope with this expression when it is processed by
extract_address_regs() because when the LO_SUM is inspected
ad->disp_loc is already non-NULL triggering this assertion:

        /* If this machine only allows one register per address, it                                     
           must be in the first operand.  */
        if (MAX_REGS_PER_ADDRESS == 1 || code == LO_SUM)
          {
=>          lra_assert (ad->disp_loc == NULL);
            ad->disp_loc = arg1_loc;
            extract_loc_address_regs (false, mode, as, arg0_loc, false, code,
                                      code1, modify_p, ad);
          }

Re: LRA has been merged into trunk.

[Marc Glisse]

On Tue, 23 Oct 2012, Vladimir Makarov wrote:

>  Hi, I was going to merge LRA into trunk last Sunday.  It did not happen. 
> LRA was actively changed last 4 weeks by implementing reviewer's proposals 
> which resulted in a lot of new LRA regressions on GCC testsuite in comparison 
> with reload.  Finally, they were fixed and everything looks ok to me.

That's great, thanks!

Note that bootstrap with java enabled is broken for me on x86_64-linux, 
I'll file a PR tonight if nobody beats me to it.

libtool: compile:  /tmp/testgcc/pristine/build/./gcc/xgcc -B/tmp/testgcc/pristine/build/./gcc/ -B/tmp/testgcc/pristine/inst/x86_64-unknown-linux-gnu/bin/ -B/tmp/testgcc/pristine/inst/x86_64-unknown-linux-gnu/lib/ -isystem /tmp/testgcc/pristine/inst/x86_64-unknown-linux-gnu/include -isystem /tmp/testgcc/pristine/inst/x86_64-unknown-linux-gnu/sys-include -DHAVE_CONFIG_H -I. -I/data/repos/gcc/pristine/libgfortran -iquote/data/repos/gcc/pristine/libgfortran/io -I/data/repos/gcc/pristine/libgfortran/../gcc -I/data/repos/gcc/pristine/libgfortran/../gcc/config -I/data/repos/gcc/pristine/libgfortran/../libquadmath -I../../.././gcc -I/data/repos/gcc/pristine/libgfortran/../libgcc -I../../libgcc -std=gnu99 -Wall -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings -fcx-fortran-rules -ffunction-sections -fdata-sections -g -O2 -m32 -MT maxloc0_4_r10.lo -MD -MP -MF .deps/maxloc0_4_r10.Tpo -c /data/repos/gcc/pristine/libgfortran/generated/maxloc0_4_r10.c  -fPIC -DPIC -o .libs/maxloc0_4_r10.o
/data/repos/gcc/pristine/libjava/classpath/gnu/xml/dom/DomNode.java: In class 'gnu.xml.dom.DomNode':
/data/repos/gcc/pristine/libjava/classpath/gnu/xml/dom/DomNode.java: In method 'gnu.xml.dom.DomNode.notifyNode(gnu.xml.dom.DomEvent,gnu.xml.dom.DomNode,boolean,gnu.xml.dom.DomNode$ListenerRecord[])':
In file included from /data/repos/gcc/pristine/libjava/classpath/gnu/xml/dom/ls/DomLSException.java:55:0,
                  from /data/repos/gcc/pristine/libjava/classpath/gnu/xml/dom/ls/DomLSInput.java:157,
                  from /data/repos/gcc/pristine/libjava/classpath/gnu/xml/dom/ls/DomLSOutput.java:96,
                  from /data/repos/gcc/pristine/libjava/classpath/gnu/xml/dom/ls/SAXEventSink.java:601,
                  from /data/repos/gcc/pristine/libjava/classpath/gnu/xml/dom/ls/WriterOutputStream.java:95,
                  from /data/repos/gcc/pristine/libjava/classpath/gnu/xml/dom/ls/FilteredSAXEventSink.java:350,
                  from /data/repos/gcc/pristine/libjava/classpath/gnu/xml/dom/ls/DomLSParser.java:565,
                  from /data/repos/gcc/pristine/libjava/classpath/gnu/xml/dom/ls/DomLSSerializer.java:350,
                  from /data/repos/gcc/pristine/libjava/classpath/gnu/xml/dom/ls/ReaderInputStream.java:233,
                  from <built-in>:112:
/data/repos/gcc/pristine/libjava/classpath/gnu/xml/dom/DomNode.java:1752:0: internal compiler error: Segmentation fault
    }
  ^
0x89757f crash_signal
         /data/repos/gcc/pristine/gcc/toplev.c:335
0x7b4be6 lra_inheritance()
         /data/repos/gcc/pristine/gcc/basic-block.h:590
0x7a6851 lra(_IO_FILE*)
         /data/repos/gcc/pristine/gcc/lra.c:2296
0x76ebe6 do_reload
         /data/repos/gcc/pristine/gcc/ira.c:4613
0x76ebe6 rest_of_handle_reload
         /data/repos/gcc/pristine/gcc/ira.c:4719



-- 
Marc Glisse

Re: LRA has been merged into trunk.

[Matthias Klose]

On 24.10.2012 08:55, Marc Glisse wrote:
> On Tue, 23 Oct 2012, Vladimir Makarov wrote:
> 
>>  Hi, I was going to merge LRA into trunk last Sunday.  It did not happen. LRA
>> was actively changed last 4 weeks by implementing reviewer's proposals which
>> resulted in a lot of new LRA regressions on GCC testsuite in comparison with
>> reload.  Finally, they were fixed and everything looks ok to me.
> 
> That's great, thanks!
> 
> Note that bootstrap with java enabled is broken for me on x86_64-linux, I'll
> file a PR tonight if nobody beats me to it.

PR55048, seen on i686-linux-gnu as well.

Re: LRA has been merged into trunk.

[Richard Sandiford]

David Miller <davem@davemloft.net> writes:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 23 Oct 2012 21:44:05 -0400 (EDT)
>
>> The first issue sparc runs into is that it does not define it's
>> extra constraints properly.  In particular 'T' and 'W' must use
>> define_memory_constraint.
>> 
>> Otherwise the EXTRA_MEMORY_CONSTRAINT logic in process_alt_operands()
>> does not trigger and we therefore cannot even load a constant into
>> floating point registers properly.
>
> Ok, the next problem we hit will be a little bit more difficult
> to solve.
>
> Sparc accepts addresses of the form:
>
> (plus:DI (lo_sum:DI (reg/f:DI 282)
>         (symbol_ref:DI ("__mf_opts") <var_decl 0xf78d74a0 __mf_opts>))
>     (const_int 40 [0x28]))
>
> These make use of Sparc's offsetable %lo() relocations.

Hmm, this looks a bit risky.  In terms of RTL semantics, this
(plus:DI ...) is a full 64-bit addition of the result of the
(lo_sum:DI ...), whatever that (lo_sum:DI ...) result happens to be.
I assume the offset is really folded into the %lo() constant itself,
in which case the usual form would be:

(lo_sum:DI (reg/f:DI 282)
           (const:DI (plus:DI (symbol_ref:DI ...) (const_int 40))))

That's the form created by e.g. adjust_address_1 in cases where it can
prove that adding the offset won't trigger a carry into the high part:

      /* If MEMREF is a LO_SUM and the offset is within the alignment of the
	 object, we can merge it into the LO_SUM.  */
      if (GET_MODE (memref) != BLKmode && GET_CODE (addr) == LO_SUM
	  && offset >= 0
	  && (unsigned HOST_WIDE_INT) offset
	      < GET_MODE_ALIGNMENT (GET_MODE (memref)) / BITS_PER_UNIT)
	addr = gen_rtx_LO_SUM (address_mode, XEXP (addr, 0),
			       plus_constant (address_mode,
					      XEXP (addr, 1), offset));

(which, now I'm quoting it, probably isn't correct for extreme
alignments, but still).

In other words, the LO_SUM constant can have an extra offset over and
above the HIGH constant in cases where we "know" that adding the extra
offset doesn't trigger a carry.

The danger with matching PLUSes of LO_SUMs is that it would be valid
for combine to turn:

    (set (reg:DI X1) (high:DI (symbol_ref:DI "foo")))
    (set (reg:DI X2) (plus:DI (reg:DI X1) (...a variable index...)))
    (set (reg:DI X3) (lo_sum:DI (reg:DI X2) (symbol_ref:DI "foo")))
    ...(mem:DI (plus:DI (reg:DI X3) (const_int 128))) ...

into:

    (set (reg:DI X1) (high:DI (symbol_ref:DI "foo")))
    (set (reg:DI X2) (plus:DI (reg:DI X1) (...a variable index...)))
    ...(mem:DI (plus:DI (lo_sum:DI (reg:DI X2) (symbol_ref:DI "foo"))
			(const_int 128)))...

even in cases where foo+128 triggers a carry into the high-part relocation.
I assume that would produce wrong code, since the carry won't be
represented in the assignment to X1.  On the other hand, combine
couldn't transform this to:

    (set (reg:DI X1) (high:DI (symbol_ref:DI "foo")))
    (set (reg:DI X2) (plus:DI (reg:DI X1) (...a variable index...)))
    ...(mem:DI (lo_sum:DI (reg:DI X2) (const:DI (plus:DI (symbol_ref:DI "foo"))
						(const_int 128)))) ...

without proving the lack of a carry.

FWIW, MIPS just accepts the (lo_sum ... (const ...)) form and I've
not noticed any problems.  In particular, doubleword loads do use:

	lui	$4,%hi(foo)
	lw	$6,%lo(foo)($4)
	lw	$7,%lo(foo+4)($4)

as hoped.  There might well be other rtl optimisations that need
to know about this though.

I suppose this comes back to what Vlad was saying about the choice
between (a) trying to make LRA work with current backends as far
as possible and (b) not adding workarounds or complications to
LRA in cases where other code really ought to change instead.
The reviews I did were definitely pushing in the (b) direction.
(Although in fairness, I think the original LRA code could have
miscompiled this kind of thing instead of aborting, because the
disp_loc would have been set to one constant or the other.)

Richard

Re: LRA has been merged into trunk.

[Jakub Jelinek]

On Wed, Oct 24, 2012 at 10:17:48AM +0100, Richard Sandiford wrote:
> > Sparc accepts addresses of the form:
> >
> > (plus:DI (lo_sum:DI (reg/f:DI 282)
> >         (symbol_ref:DI ("__mf_opts") <var_decl 0xf78d74a0 __mf_opts>))
> >     (const_int 40 [0x28]))
> >
> > These make use of Sparc's offsetable %lo() relocations.
> 
> Hmm, this looks a bit risky.  In terms of RTL semantics, this
> (plus:DI ...) is a full 64-bit addition of the result of the
> (lo_sum:DI ...), whatever that (lo_sum:DI ...) result happens to be.
> I assume the offset is really folded into the %lo() constant itself,
> in which case the usual form would be:
> 
> (lo_sum:DI (reg/f:DI 282)
>            (const:DI (plus:DI (symbol_ref:DI ...) (const_int 40))))

R_SPARC_OLO10 relocation has two addends though, please see
http://sourceware.org/ml/binutils/1999-q3/msg00099.html
so the plus there is right.  The constant in second operand of PLUS
where first operand is LO_SUM needs to be RTX_OK_FOR_OLO10_P, so that
there is no overflow on the lo_sum operand & 0x3ff plus the
RTX_OK_FOR_OLO10_P offset (it needs to fit into signed 13 bit field).

	Jakub

Re: LRA has been merged into trunk.

[Richard Sandiford]

Jakub Jelinek <jakub@redhat.com> writes:
> On Wed, Oct 24, 2012 at 10:17:48AM +0100, Richard Sandiford wrote:
>> > Sparc accepts addresses of the form:
>> >
>> > (plus:DI (lo_sum:DI (reg/f:DI 282)
>> >         (symbol_ref:DI ("__mf_opts") <var_decl 0xf78d74a0 __mf_opts>))
>> >     (const_int 40 [0x28]))
>> >
>> > These make use of Sparc's offsetable %lo() relocations.
>> 
>> Hmm, this looks a bit risky.  In terms of RTL semantics, this
>> (plus:DI ...) is a full 64-bit addition of the result of the
>> (lo_sum:DI ...), whatever that (lo_sum:DI ...) result happens to be.
>> I assume the offset is really folded into the %lo() constant itself,
>> in which case the usual form would be:
>> 
>> (lo_sum:DI (reg/f:DI 282)
>>            (const:DI (plus:DI (symbol_ref:DI ...) (const_int 40))))
>
> R_SPARC_OLO10 relocation has two addends though, please see
> http://sourceware.org/ml/binutils/1999-q3/msg00099.html
> so the plus there is right.  The constant in second operand of PLUS
> where first operand is LO_SUM needs to be RTX_OK_FOR_OLO10_P, so that
> there is no overflow on the lo_sum operand & 0x3ff plus the
> RTX_OK_FOR_OLO10_P offset (it needs to fit into signed 13 bit field).

OK, that blasts that argument out of the water then. :-)

Richard

Re: LRA has been merged into trunk.

[Ramana Radhakrishnan]

On 10/23/12 16:46, Vladimir Makarov wrote:
>     Hi, I was going to merge LRA into trunk last Sunday.  It did not
> happen.  LRA was actively changed last 4 weeks by implementing
> reviewer's proposals which resulted in a lot of new LRA regressions on
> GCC testsuite in comparison with reload.  Finally, they were fixed and
> everything looks ok to me.
>
>     So I've committed the patch into trunk as rev. 192719. The final
> patch is in the attachment.
>

This prima-facie caused PR55050 on arm-linux-gnueabi . CC'd you on the 
bug report.


regards,
Ramana

Re: LRA has been merged into trunk.

[H.J. Lu]

On Tue, Oct 23, 2012 at 8:46 AM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>   Hi, I was going to merge LRA into trunk last Sunday.  It did not happen.
> LRA was actively changed last 4 weeks by implementing reviewer's proposals
> which resulted in a lot of new LRA regressions on GCC testsuite in
> comparison with reload.  Finally, they were fixed and everything looks ok to
> me.
>
>   So I've committed the patch into trunk as rev. 192719. The final patch is
> in the attachment.
>

It caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55049

-- 
H.J.

Re: LRA has been merged into trunk.

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 23 Oct 2012 22:06:55 -0400 (EDT)

> From: David Miller <davem@davemloft.net>
> Date: Tue, 23 Oct 2012 21:44:05 -0400 (EDT)
> 
>> The first issue sparc runs into is that it does not define it's
>> extra constraints properly.  In particular 'T' and 'W' must use
>> define_memory_constraint.
>> 
>> Otherwise the EXTRA_MEMORY_CONSTRAINT logic in process_alt_operands()
>> does not trigger and we therefore cannot even load a constant into
>> floating point registers properly.
> 
> Ok, the next problem we hit will be a little bit more difficult
> to solve.
> 
> Sparc accepts addresses of the form:
> 
> (plus:DI (lo_sum:DI (reg/f:DI 282)
>         (symbol_ref:DI ("__mf_opts") <var_decl 0xf78d74a0 __mf_opts>))
>     (const_int 40 [0x28]))
> 
> These make use of Sparc's offsetable %lo() relocations.
> 
> But LRA is unable to cope with this expression when it is processed by
> extract_address_regs() because when the LO_SUM is inspected
> ad->disp_loc is already non-NULL triggering this assertion:

So I added a workaround for this, and the next problem we hit involves
PIC.  It stems from the HAVE_lo_sum code block in process_address.

It unconditionally tries to perform a HIGH/LO_SUM sequence to load an
address, and depends upon insn recognition and target backend address
validation to reject such sequences as needed.

Actually, this HAVE_lo_sum code block seems to be an ad-hoc
replacement for a target macro, namely LEGITIMIZE_RELOAD_ADDRESS.

However, on Sparc, LEGITIMIZE_RELOAD_ADDRESS would never emit the
HIGH/LO_SUM sequence for PIC.  Instead, it would leave it to reload to
push the reload and then emit a move of the address into a register,
which in turn would go through all the proper PIC handling logic in
the Sparc backend's move expander.

Before LRA, the target legitimate_address_p would never have to ever
see such strange "(LO_SUM REG SYMBOLIC)" expressions when PIC is
enabled, but now it can and they therefore have to now be explicitly
checked for.

This case triggers every time LRA does a force_const_mem().

I'll add the straightforward check to sparc's legitimate_address_p,
but I'm really surprised you never hit this case in your testing.

At this point I have the C testsuite regressions down to about 20
failures.

Re: LRA has been merged into trunk.

[Richard Sandiford]

David Miller <davem@davemloft.net> writes:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 23 Oct 2012 22:06:55 -0400 (EDT)
>
>> From: David Miller <davem@davemloft.net>
>> Date: Tue, 23 Oct 2012 21:44:05 -0400 (EDT)
>> 
>>> The first issue sparc runs into is that it does not define it's
>>> extra constraints properly.  In particular 'T' and 'W' must use
>>> define_memory_constraint.
>>> 
>>> Otherwise the EXTRA_MEMORY_CONSTRAINT logic in process_alt_operands()
>>> does not trigger and we therefore cannot even load a constant into
>>> floating point registers properly.
>> 
>> Ok, the next problem we hit will be a little bit more difficult
>> to solve.
>> 
>> Sparc accepts addresses of the form:
>> 
>> (plus:DI (lo_sum:DI (reg/f:DI 282)
>>         (symbol_ref:DI ("__mf_opts") <var_decl 0xf78d74a0 __mf_opts>))
>>     (const_int 40 [0x28]))
>> 
>> These make use of Sparc's offsetable %lo() relocations.
>> 
>> But LRA is unable to cope with this expression when it is processed by
>> extract_address_regs() because when the LO_SUM is inspected
>> ad->disp_loc is already non-NULL triggering this assertion:
>
> So I added a workaround for this, and the next problem we hit involves
> PIC.  It stems from the HAVE_lo_sum code block in process_address.
>
> It unconditionally tries to perform a HIGH/LO_SUM sequence to load an
> address, and depends upon insn recognition and target backend address
> validation to reject such sequences as needed.
>
> Actually, this HAVE_lo_sum code block seems to be an ad-hoc
> replacement for a target macro, namely LEGITIMIZE_RELOAD_ADDRESS.
>
> However, on Sparc, LEGITIMIZE_RELOAD_ADDRESS would never emit the
> HIGH/LO_SUM sequence for PIC.  Instead, it would leave it to reload to
> push the reload and then emit a move of the address into a register,
> which in turn would go through all the proper PIC handling logic in
> the Sparc backend's move expander.
>
> Before LRA, the target legitimate_address_p would never have to ever
> see such strange "(LO_SUM REG SYMBOLIC)" expressions when PIC is
> enabled, but now it can and they therefore have to now be explicitly
> checked for.

I think that's a true feature rather than an ad-hoc feature or bug.
If the target supports LO_SUM then this split is an obvious thing to try.

FWIW, combine.c:find_split_point does a similar thing in a similar
situation:

#ifdef HAVE_lo_sum
      /* If we have (mem (const ..)) or (mem (symbol_ref ...)), split it
	 using LO_SUM and HIGH.  */
      if (GET_CODE (XEXP (x, 0)) == CONST
	  || GET_CODE (XEXP (x, 0)) == SYMBOL_REF)
	{
	  enum machine_mode address_mode = get_address_mode (x);

	  SUBST (XEXP (x, 0),
		 gen_rtx_LO_SUM (address_mode,
				 gen_rtx_HIGH (address_mode, XEXP (x, 0)),
				 XEXP (x, 0)));
	  return &XEXP (XEXP (x, 0), 0);
	}
#endif

> This case triggers every time LRA does a force_const_mem().
>
> I'll add the straightforward check to sparc's legitimate_address_p,
> but I'm really surprised you never hit this case in your testing.

Adding the check sounds like the right thing to do.  And remember
that the merge was only for the core LRA code and x86 bits.
The LRA branch has patches to e.g. rs6000's address handling;
it's just something that needs to be done when "porting" to LRA.

Richard

Re: LRA has been merged into trunk.

[David Miller]

From: Richard Sandiford <rdsandiford@googlemail.com>
Date: Thu, 25 Oct 2012 16:34:00 +0100

> David Miller <davem@davemloft.net> writes:
>> I'll add the straightforward check to sparc's legitimate_address_p,
>> but I'm really surprised you never hit this case in your testing.
> 
> Adding the check sounds like the right thing to do.  And remember
> that the merge was only for the core LRA code and x86 bits.
> The LRA branch has patches to e.g. rs6000's address handling;
> it's just something that needs to be done when "porting" to LRA.

Fair enough.

FWIW, I currently have the whole 32-bit sparc build and testsuite down
to no regressions.

Re: LRA has been merged into trunk.

[H.J. Lu]

On Wed, Oct 24, 2012 at 3:16 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Oct 23, 2012 at 8:46 AM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>>   Hi, I was going to merge LRA into trunk last Sunday.  It did not happen.
>> LRA was actively changed last 4 weeks by implementing reviewer's proposals
>> which resulted in a lot of new LRA regressions on GCC testsuite in
>> comparison with reload.  Finally, they were fixed and everything looks ok to
>> me.
>>
>>   So I've committed the patch into trunk as rev. 192719. The final patch is
>> in the attachment.
>>
>
> It caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55049
>

Is there a command line option to turn off LRA from command-line to
compare code quality?

Thanks.


-- 
H.J.

Re: LRA has been merged into trunk.

[David Miller]

From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 25 Oct 2012 22:59:58 -0700

> On Wed, Oct 24, 2012 at 3:16 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Oct 23, 2012 at 8:46 AM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>>>   Hi, I was going to merge LRA into trunk last Sunday.  It did not happen.
>>> LRA was actively changed last 4 weeks by implementing reviewer's proposals
>>> which resulted in a lot of new LRA regressions on GCC testsuite in
>>> comparison with reload.  Finally, they were fixed and everything looks ok to
>>> me.
>>>
>>>   So I've committed the patch into trunk as rev. 192719. The final patch is
>>> in the attachment.
>>>
>>
>> It caused:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55049
>>
> 
> Is there a command line option to turn off LRA from command-line to
> compare code quality?

Currently no.

Re: LRA has been merged into trunk.

[H.J. Lu]

On Thu, Oct 25, 2012 at 11:12 PM, David Miller <davem@davemloft.net> wrote:
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Thu, 25 Oct 2012 22:59:58 -0700
>
>> On Wed, Oct 24, 2012 at 3:16 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Tue, Oct 23, 2012 at 8:46 AM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>>>>   Hi, I was going to merge LRA into trunk last Sunday.  It did not happen.
>>>> LRA was actively changed last 4 weeks by implementing reviewer's proposals
>>>> which resulted in a lot of new LRA regressions on GCC testsuite in
>>>> comparison with reload.  Finally, they were fixed and everything looks ok to
>>>> me.
>>>>
>>>>   So I've committed the patch into trunk as rev. 192719. The final patch is
>>>> in the attachment.
>>>>
>>>
>>> It caused:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55049
>>>
>>
>> Is there a command line option to turn off LRA from command-line to
>> compare code quality?
>
> Currently no.

Should there be a -fno-ira option before reload pass is
removed?  It will be useful to investiage IRA regressions.

-- 
H.J.

Re: LRA has been merged into trunk.

[Jakub Jelinek]

On Thu, Oct 25, 2012 at 11:15:06PM -0700, H.J. Lu wrote:
> Should there be a -fno-ira option before reload pass is
> removed?  It will be useful to investiage IRA regressions.

You mean -fno-lra, and s/IRA/LRA/, right?  I think the reason for no
compiler switch is that while returning false from ix86_lra_p ()
likely works right now, -fno-lra mode would be yet another
thing to support.  So, for investigations just return false from ix86_lra_p,
similarly for benchmarking, but as it needs compiler source changes, it is
obvious that with old reload everybody is on their own if it breaks
for targets that switched to LRA.

	Jakub

Re: LRA has been merged into trunk.

[H.J. Lu]

On Thu, Oct 25, 2012 at 11:21 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Oct 25, 2012 at 11:15:06PM -0700, H.J. Lu wrote:
>> Should there be a -fno-ira option before reload pass is
>> removed?  It will be useful to investiage IRA regressions.
>
> You mean -fno-lra, and s/IRA/LRA/, right?  I think the reason for no

Yes, I meant -fno-lra.

> compiler switch is that while returning false from ix86_lra_p ()
> likely works right now, -fno-lra mode would be yet another
> thing to support.  So, for investigations just return false from ix86_lra_p,
> similarly for benchmarking, but as it needs compiler source changes, it is
> obvious that with old reload everybody is on their own if it breaks
> for targets that switched to LRA.
>

I'd like to compare glibc code quality on x32, x86-64 and ia32
with and without LRA.  We don't even need to document -fno-lra
or we can make it -mno-lra as x86 undocumented switch.  I
expect -fno-lra/-mno-lra will only be useful in a short period
of time.


-- 
H.J.

Re: LRA has been merged into trunk.

[Richard Biener]

On Fri, Oct 26, 2012 at 8:30 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Oct 25, 2012 at 11:21 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Thu, Oct 25, 2012 at 11:15:06PM -0700, H.J. Lu wrote:
>>> Should there be a -fno-ira option before reload pass is
>>> removed?  It will be useful to investiage IRA regressions.
>>
>> You mean -fno-lra, and s/IRA/LRA/, right?  I think the reason for no
>
> Yes, I meant -fno-lra.
>
>> compiler switch is that while returning false from ix86_lra_p ()
>> likely works right now, -fno-lra mode would be yet another
>> thing to support.  So, for investigations just return false from ix86_lra_p,
>> similarly for benchmarking, but as it needs compiler source changes, it is
>> obvious that with old reload everybody is on their own if it breaks
>> for targets that switched to LRA.
>>
>
> I'd like to compare glibc code quality on x32, x86-64 and ia32
> with and without LRA.  We don't even need to document -fno-lra
> or we can make it -mno-lra as x86 undocumented switch.  I
> expect -fno-lra/-mno-lra will only be useful in a short period
> of time.

You can always compare to the revision before LRA merged.  My understanding
is that -fno-lra is not easily possible as backends change in non-trivial
ways once they go the LRA way.  Thus you'd not compare what you want
to compare.

Richard.

>
> --
> H.J.

Re: LRA has been merged into trunk.

[H.J. Lu]

On Fri, Oct 26, 2012 at 2:54 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Fri, Oct 26, 2012 at 8:30 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, Oct 25, 2012 at 11:21 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>> On Thu, Oct 25, 2012 at 11:15:06PM -0700, H.J. Lu wrote:
>>>> Should there be a -fno-ira option before reload pass is
>>>> removed?  It will be useful to investiage IRA regressions.
>>>
>>> You mean -fno-lra, and s/IRA/LRA/, right?  I think the reason for no
>>
>> Yes, I meant -fno-lra.
>>
>>> compiler switch is that while returning false from ix86_lra_p ()
>>> likely works right now, -fno-lra mode would be yet another
>>> thing to support.  So, for investigations just return false from ix86_lra_p,
>>> similarly for benchmarking, but as it needs compiler source changes, it is
>>> obvious that with old reload everybody is on their own if it breaks
>>> for targets that switched to LRA.
>>>
>>
>> I'd like to compare glibc code quality on x32, x86-64 and ia32
>> with and without LRA.  We don't even need to document -fno-lra
>> or we can make it -mno-lra as x86 undocumented switch.  I
>> expect -fno-lra/-mno-lra will only be useful in a short period
>> of time.
>
> You can always compare to the revision before LRA merged.  My understanding
> is that -fno-lra is not easily possible as backends change in non-trivial
> ways once they go the LRA way.  Thus you'd not compare what you want
> to compare.
>

I created hjl/lra git branch and added -mno-lra to x86 backend.
For now, one can use it to compare LRA vs reload code quality
on x86..

-- 
H.J.

Re: LRA has been merged into trunk.

[David Edelsohn]

On Wed, Oct 24, 2012 at 8:08 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
> And PR bootstrap/55068 due to assert failure in push_reload() .

GCC bootstrapped on AIX with your patches.  Thanks for fixing the
problems so quickly.

- David

Re: LRA has been merged into trunk.

[David Edelsohn]

And PR bootstrap/55068 due to assert failure in push_reload() .

Thanks, David

Re: LRA has been merged into trunk.

[David Edelsohn]

This also causes PR bootstrap/55067 on AIX due to the use of typedef loc_t.

Thanks, David

Re: LRA has been merged into trunk.

[Vladimir Makarov]

On 10/23/2012 01:57 PM, Uros Bizjak wrote:
> Hello!
>
>> Hi, I was going to merge LRA into trunk last Sunday. It did not happen. LRA was actively changed last 4 weeks by implementing reviewer's proposals which resulted in a lot of new LRA regressions on GCC testsuite in comparison with reload. Finally, they were fixed and everything looks ok to me.
>> So I've committed the patch into trunk as rev. 192719. The final patch is in the attachment.
> This commit introduced following testsuite failure on
> x86_64-pc-linux-gnu with -m32:
>
> FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90,  -O2
> (internal compiler error)
> FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90,  -O2
> -fomit-frame-pointer -finline-functions  (internal compiler error)
> FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90,  -O2
> -fomit-frame-pointer -finline-functions -funroll-loops  (internal
> compiler error)
> FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90,  -O2
> -fbounds-check  (internal compiler error)
> FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90,  -O3 -g
> (internal compiler error)
> FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90,  -Os
> (internal compiler error)
> FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90, -O2
> -ftree-vectorize -msse2  (internal compiler error)
>
>
Thanks, Uros.  I'll be working on this.  I tested with -march=corei7 and 
-mtune=corei7 (that is what H.J. uses) therefore I did not see it.

Re: LRA has been merged into trunk.

[Uros Bizjak]

Hello!

> Hi, I was going to merge LRA into trunk last Sunday. It did not happen. LRA was actively changed last 4 weeks by implementing reviewer's proposals which resulted in a lot of new LRA regressions on GCC testsuite in comparison with reload. Finally, they were fixed and everything looks ok to me.

> So I've committed the patch into trunk as rev. 192719. The final patch is in the attachment.

This commit introduced following testsuite failure on
x86_64-pc-linux-gnu with -m32:

FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90,  -O2
(internal compiler error)
FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90,  -O2
-fomit-frame-pointer -finline-functions  (internal compiler error)
FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90,  -O2
-fomit-frame-pointer -finline-functions -funroll-loops  (internal
compiler error)
FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90,  -O2
-fbounds-check  (internal compiler error)
FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90,  -O3 -g
(internal compiler error)
FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90,  -Os
(internal compiler error)
FAIL: gfortran.fortran-torture/execute/intrinsic_nearest.f90, -O2
-ftree-vectorize -msse2  (internal compiler error)

The error is:

/home/uros/gcc-svn/trunk/gcc/testsuite/gfortran.fortran-torture/execute/intrinsic_nearest.f90:
In function 'test_n':
/home/uros/gcc-svn/trunk/gcc/testsuite/gfortran.fortran-torture/execute/intrinsic_nearest.f90:77:0:
internal compiler error: in lra_assign, at lra-assigns.c:1361
0x8557e3 lra_assign()
	../../gcc-svn/trunk/gcc/lra-assigns.c:1361
0x8538bc lra(_IO_FILE*)
	../../gcc-svn/trunk/gcc/lra.c:2310
0x81bc46 do_reload
	../../gcc-svn/trunk/gcc/ira.c:4613
0x81bc46 rest_of_handle_reload
	../../gcc-svn/trunk/gcc/ira.c:4719
Please submit a full bug report,

Uros.