| Message ID | 52711FB9.7000802@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Oct 30, 2013 at 3:03 PM, Vladimir Makarov <vmakarov@redhat.com> wrote: > The following patch fixes > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58784 > > LRA has an old check of legitimate addresses. It was written before a newer > address decomposition code which makes more correct checks of addresses. > > So I removed the old check. > > Committed as rev. 204215. > > 2013-10-30 Vladimir Makarov <vmakarov@redhat.com> > > PR target/58784 > * lra.c (check_rtl): Remove address check before LRA work. > > 2013-10-30 Vladimir Makarov <vmakarov@redhat.com> > > PR target/58784 > * gcc.target/arm/pr58784.c: New. >
On Wed, Oct 30, 2013 at 3:03 PM, Vladimir Makarov <vmakarov@redhat.com> wrote: > The following patch fixes > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58784 > > LRA has an old check of legitimate addresses. It was written before a newer > address decomposition code which makes more correct checks of addresses. > > So I removed the old check. > > Committed as rev. 204215. > Yvan, Does this mean we can now turn on LRA for ARM state by default and start looking at performance issues if any ? Ramana > 2013-10-30 Vladimir Makarov <vmakarov@redhat.com> > > PR target/58784 > * lra.c (check_rtl): Remove address check before LRA work. > > 2013-10-30 Vladimir Makarov <vmakarov@redhat.com> > > PR target/58784 > * gcc.target/arm/pr58784.c: New. >
(this time in plain text !) > Does this mean we can now turn on LRA for ARM state by default and > start looking at performance issues if any ? With the other patch for 58785 and a small one I've to post, we are even about to turn LRA on by default completely, as the compiler now bootstrap also in thumb and first testsuite are clean :-) I juste want ton pass a full validation before. Yvan >> Ramana >> >> >> >> >> >> > 2013-10-30 Vladimir Makarov <vmakarov@redhat.com> >> > >> > PR target/58784 >> > * lra.c (check_rtl): Remove address check before LRA work. >> > >> > 2013-10-30 Vladimir Makarov <vmakarov@redhat.com> >> > >> > PR target/58784 >> > * gcc.target/arm/pr58784.c: New. >> >
Hi,
I've tested LRA on ARM in several configuration and it occurs that
only one regression is observed in Fortran for targets A15 and A9 in
hard and soft fp amd Thumb mode enabled by default (ARMv5 is clean).
The failing test case is gfortran.dg/realloc_on_assign_11.f90 in -O3
and the error seems to be of the same nature of
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58784, in the sens that
LRA ICEs in check_rtl on an insn which doesn't satisfy its
constraints. Here is the insn :
(insn 633 543 656 24 (parallel [
(set (mem/c:SI (plus:SI (reg/f:SI 907)
(const_int 8 [0x8])) [4 A.54+8 S4 A64])
(smax:SI (reg:SI 562 [ D.5235 ])
(reg:SI 621 [ D.5235 ])))
(clobber (reg:CC 100 cc))
]).../gfortran.dg/realloc_on_assign_11.f90:29 121 {*store_minmaxsi}
(expr_list:REG_DEAD (reg:SI 621 [ D.5235 ])
(expr_list:REG_UNUSED (reg:CC 100 cc)
(nil))))
Thanks,
Yvan
On 31 October 2013 17:31, Yvan Roux <yvan.roux@linaro.org> wrote:
> (this time in plain text !)
>
>> Does this mean we can now turn on LRA for ARM state by default and
>> start looking at performance issues if any ?
>
> With the other patch for 58785 and a small one I've to post, we are even
> about to turn LRA on by default completely, as the compiler now bootstrap
> also in thumb and first testsuite are clean :-)
>
> I juste want ton pass a full validation before.
>
> Yvan
>
>
>>> Ramana
>>>
>>>
>>>
>>>
>>>
>>> > 2013-10-30 Vladimir Makarov <vmakarov@redhat.com>
>>> >
>>> > PR target/58784
>>> > * lra.c (check_rtl): Remove address check before LRA work.
>>> >
>>> > 2013-10-30 Vladimir Makarov <vmakarov@redhat.com>
>>> >
>>> > PR target/58784
>>> > * gcc.target/arm/pr58784.c: New.
>>> >
On Thu, Nov 7, 2013 at 10:42 AM, Yvan Roux <yvan.roux@linaro.org> wrote: > Hi, > > I've tested LRA on ARM in several configuration and it occurs that > only one regression is observed in Fortran for targets A15 and A9 in > hard and soft fp amd Thumb mode enabled by default (ARMv5 is clean). > The failing test case is gfortran.dg/realloc_on_assign_11.f90 in -O3 > and the error seems to be of the same nature of > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58784, in the sens that > LRA ICEs in check_rtl on an insn which doesn't satisfy its > constraints. Here is the insn : > > (insn 633 543 656 24 (parallel [ > (set (mem/c:SI (plus:SI (reg/f:SI 907) > (const_int 8 [0x8])) [4 A.54+8 S4 A64]) > (smax:SI (reg:SI 562 [ D.5235 ]) > (reg:SI 621 [ D.5235 ]))) > (clobber (reg:CC 100 cc)) > ]).../gfortran.dg/realloc_on_assign_11.f90:29 121 {*store_minmaxsi} > (expr_list:REG_DEAD (reg:SI 621 [ D.5235 ]) > (expr_list:REG_UNUSED (reg:CC 100 cc) > (nil)))) This store_minmaxsi insn is not recognized because optimize_function_for_size_p() is false. The problem is in the place where this insn was created. Trying to figure that out now... Ciao! Steven
Index: lra.c =================================================================== --- lra.c (revision 204131) +++ lra.c (working copy) @@ -2017,10 +2017,8 @@ static void check_rtl (bool final_p) { - int i; basic_block bb; rtx insn; - lra_insn_recog_data_t id; lra_assert (! final_p || reload_completed); FOR_EACH_BB (bb) @@ -2036,31 +2034,13 @@ lra_assert (constrain_operands (1)); continue; } + /* LRA code is based on assumption that all addresses can be + correctly decomposed. LRA can generate reloads for + decomposable addresses. The decomposition code checks the + correctness of the addresses. So we don't need to check + the addresses here. */ if (insn_invalid_p (insn, false)) fatal_insn_not_found (insn); - if (asm_noperands (PATTERN (insn)) >= 0) - continue; - id = lra_get_insn_recog_data (insn); - /* The code is based on assumption that all addresses in - regular instruction are legitimate before LRA. The code in - lra-constraints.c is based on assumption that there is no - subreg of memory as an insn operand. */ - for (i = 0; i < id->insn_static_data->n_operands; i++) - { - rtx op = *id->operand_loc[i]; - - if (MEM_P (op) - && (GET_MODE (op) != BLKmode - || GET_CODE (XEXP (op, 0)) != SCRATCH) - && ! memory_address_p (GET_MODE (op), XEXP (op, 0)) - /* Some ports don't recognize the following addresses - as legitimate. Although they are legitimate if - they satisfies the constraints and will be checked - by insn constraints which we ignore here. */ - && GET_CODE (XEXP (op, 0)) != UNSPEC - && GET_RTX_CLASS (GET_CODE (XEXP (op, 0))) != RTX_AUTOINC) - fatal_insn_not_found (insn); - } } } #endif /* #ifdef ENABLE_CHECKING */ Index: testsuite/gcc.target/arm/pr58784.c =================================================================== --- testsuite/gcc.target/arm/pr58784.c (revision 0) +++ testsuite/gcc.target/arm/pr58784.c (working copy) @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-march=armv7-a -mfloat-abi=hard -mfpu=neon -marm -O2" } */ + +typedef struct __attribute__ ((__packed__)) +{ + char valueField[2]; +} ptp_tlv_t; +typedef struct __attribute__ ((__packed__)) +{ + char stepsRemoved; + ptp_tlv_t tlv[1]; +} ptp_message_announce_t; +int ptplib_send_announce(int sequenceId, int i) +{ + ptp_message_announce_t tx_packet; + ((long long *)tx_packet.tlv[0].valueField)[sequenceId] = i; + f(&tx_packet); +}
The following patch fixes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58784 LRA has an old check of legitimate addresses. It was written before a newer address decomposition code which makes more correct checks of addresses. So I removed the old check. Committed as rev. 204215. 2013-10-30 Vladimir Makarov <vmakarov@redhat.com> PR target/58784 * lra.c (check_rtl): Remove address check before LRA work. 2013-10-30 Vladimir Makarov <vmakarov@redhat.com> PR target/58784 * gcc.target/arm/pr58784.c: New.