Re: LRA has been merged into trunk.
On Wed, Oct 24, 2012 at 3:16 AM, H.J. Lu hjl.to...@gmail.com wrote: On Tue, Oct 23, 2012 at 8:46 AM, Vladimir Makarov vmaka...@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.
From: H.J. Lu hjl.to...@gmail.com Date: Thu, 25 Oct 2012 22:59:58 -0700 On Wed, Oct 24, 2012 at 3:16 AM, H.J. Lu hjl.to...@gmail.com wrote: On Tue, Oct 23, 2012 at 8:46 AM, Vladimir Makarov vmaka...@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.
On Thu, Oct 25, 2012 at 11:12 PM, David Miller da...@davemloft.net wrote: From: H.J. Lu hjl.to...@gmail.com Date: Thu, 25 Oct 2012 22:59:58 -0700 On Wed, Oct 24, 2012 at 3:16 AM, H.J. Lu hjl.to...@gmail.com wrote: On Tue, Oct 23, 2012 at 8:46 AM, Vladimir Makarov vmaka...@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.
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.
On Thu, Oct 25, 2012 at 11:21 PM, Jakub Jelinek ja...@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.
On Fri, Oct 26, 2012 at 8:30 AM, H.J. Lu hjl.to...@gmail.com wrote: On Thu, Oct 25, 2012 at 11:21 PM, Jakub Jelinek ja...@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.
On Fri, Oct 26, 2012 at 2:54 AM, Richard Biener richard.guent...@gmail.com wrote: On Fri, Oct 26, 2012 at 8:30 AM, H.J. Lu hjl.to...@gmail.com wrote: On Thu, Oct 25, 2012 at 11:21 PM, Jakub Jelinek ja...@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 Miller da...@davemloft.net writes:
From: David Miller da...@davemloft.net
Date: Tue, 23 Oct 2012 22:06:55 -0400 (EDT)
From: David Miller da...@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.
From: Richard Sandiford rdsandif...@googlemail.com Date: Thu, 25 Oct 2012 16:34:00 +0100 David Miller da...@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.
On Wed, Oct 24, 2012 at 8:08 PM, David Edelsohn dje@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.
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.
And PR bootstrap/55068 due to assert failure in push_reload() . Thanks, David
Re: LRA has been merged into trunk.
From: David Miller da...@davemloft.net Date: Tue, 23 Oct 2012 22:06:55 -0400 (EDT) From: David Miller da...@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.
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.
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.
David Miller da...@davemloft.net writes: From: David Miller da...@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.
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.
Jakub Jelinek ja...@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.
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.
On Tue, Oct 23, 2012 at 8:46 AM, Vladimir Makarov vmaka...@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.
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.
Re: LRA has been merged into trunk.
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.
From: Vladimir Makarov vmaka...@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.
On 12-10-23 5:28 PM, David Miller wrote: From: Vladimir Makarov vmaka...@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.
From: Vladimir Makarov vmaka...@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.
From: David Miller da...@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);
}
