| Message ID | 51B37A3F.1060705@redhat.com |
|---|---|
| State | New |
| Headers | show |
Vladimir Makarov <vmakarov@redhat.com> writes: > Index: lra.c > =================================================================== > --- lra.c (revision 199753) > +++ lra.c (working copy) > @@ -306,11 +306,11 @@ lra_emit_add (rtx x, rtx y, rtx z) > || (disp != NULL_RTX && ! CONSTANT_P (disp)) > || (scale != NULL_RTX && ! CONSTANT_P (scale))) > { > - /* Its is not an address generation. Probably we have no 3 op > + /* It is not an address generation. Probably we have no 3 op > add. Last chance is to use 2-op add insn. */ > lra_assert (x != y && x != z); > - emit_move_insn (x, z); > - insn = gen_add2_insn (x, y); > + emit_move_insn (x, y); > + insn = gen_add2_insn (x, z); > emit_insn (insn); > } > else Could you add a comment to lra_emit_add saying why it has to be this way round (move y, add z)? Thanks, Richard
Richard Sandiford <rdsandiford@googlemail.com> writes: > Vladimir Makarov <vmakarov@redhat.com> writes: >> Index: lra.c >> =================================================================== >> --- lra.c (revision 199753) >> +++ lra.c (working copy) >> @@ -306,11 +306,11 @@ lra_emit_add (rtx x, rtx y, rtx z) >> || (disp != NULL_RTX && ! CONSTANT_P (disp)) >> || (scale != NULL_RTX && ! CONSTANT_P (scale))) >> { >> - /* Its is not an address generation. Probably we have no 3 op >> + /* It is not an address generation. Probably we have no 3 op >> add. Last chance is to use 2-op add insn. */ >> lra_assert (x != y && x != z); >> - emit_move_insn (x, z); >> - insn = gen_add2_insn (x, y); >> + emit_move_insn (x, y); >> + insn = gen_add2_insn (x, z); >> emit_insn (insn); >> } >> else > > Could you add a comment to lra_emit_add saying why it has to be this > way round (move y, add z)? Ping.
On 13-06-19 2:31 PM, Richard Sandiford wrote: > Richard Sandiford <rdsandiford@googlemail.com> writes: >> Vladimir Makarov <vmakarov@redhat.com> writes: >>> Index: lra.c >>> =================================================================== >>> --- lra.c (revision 199753) >>> +++ lra.c (working copy) >>> @@ -306,11 +306,11 @@ lra_emit_add (rtx x, rtx y, rtx z) >>> || (disp != NULL_RTX && ! CONSTANT_P (disp)) >>> || (scale != NULL_RTX && ! CONSTANT_P (scale))) >>> { >>> - /* Its is not an address generation. Probably we have no 3 op >>> + /* It is not an address generation. Probably we have no 3 op >>> add. Last chance is to use 2-op add insn. */ >>> lra_assert (x != y && x != z); >>> - emit_move_insn (x, z); >>> - insn = gen_add2_insn (x, y); >>> + emit_move_insn (x, y); >>> + insn = gen_add2_insn (x, z); >>> emit_insn (insn); >>> } >>> else >> Could you add a comment to lra_emit_add saying why it has to be this >> way round (move y, add z)? > Ping. I am going to add a comment when I submit my next patch (it will happen today or tomorrow). The reason is simple as address segment is stored in y not in z and generation of addition of address segment to pseudo can fail (that is what happens for the PR). Thanks, Richard.
Vladimir Makarov <vmakarov@redhat.com> writes: > On 13-06-19 2:31 PM, Richard Sandiford wrote: >> Richard Sandiford <rdsandiford@googlemail.com> writes: >>> Vladimir Makarov <vmakarov@redhat.com> writes: >>>> Index: lra.c >>>> =================================================================== >>>> --- lra.c (revision 199753) >>>> +++ lra.c (working copy) >>>> @@ -306,11 +306,11 @@ lra_emit_add (rtx x, rtx y, rtx z) >>>> || (disp != NULL_RTX && ! CONSTANT_P (disp)) >>>> || (scale != NULL_RTX && ! CONSTANT_P (scale))) >>>> { >>>> - /* Its is not an address generation. Probably we have no 3 op >>>> + /* It is not an address generation. Probably we have no 3 op >>>> add. Last chance is to use 2-op add insn. */ >>>> lra_assert (x != y && x != z); >>>> - emit_move_insn (x, z); >>>> - insn = gen_add2_insn (x, y); >>>> + emit_move_insn (x, y); >>>> + insn = gen_add2_insn (x, z); >>>> emit_insn (insn); >>>> } >>>> else >>> Could you add a comment to lra_emit_add saying why it has to be this >>> way round (move y, add z)? >> Ping. > I am going to add a comment when I submit my next patch (it will happen > today or tomorrow). Thanks. > The reason is simple as address segment is stored in y not in z and > generation of addition of address segment to pseudo can fail (that is > what happens for the PR). Do you mean address segment in the x86 sense of "segment"? I was just a bit confused because the current comment says "It is not an address generation", whereas it sounds like addresses are involved somewhere. I suppose the commutation rules are that Y should be "no less complicated" than Z, so maybe it wins from that point of view too. Richard
Index: lra-constraints.c =================================================================== --- lra-constraints.c (revision 199762) +++ lra-constraints.c (working copy) @@ -1997,15 +1997,8 @@ process_alt_operands (int only_alternati (op, this_alternative) == NO_REGS)))) reject += LRA_MAX_REJECT; - if (MEM_P (op) && offmemok) - { - /* If we know offset and this non-offsetable memory, - something wrong with this memory and it is better - to try other memory possibilities. */ - if (MEM_OFFSET_KNOWN_P (op)) - reject += LRA_MAX_REJECT; - } - else if (! (const_to_mem && constmemok)) + if (! (MEM_P (op) && offmemok) + && ! (const_to_mem && constmemok)) { /* We prefer to reload pseudos over reloading other things, since such reloads may be able to be Index: lra.c =================================================================== --- lra.c (revision 199753) +++ lra.c (working copy) @@ -306,11 +306,11 @@ lra_emit_add (rtx x, rtx y, rtx z) || (disp != NULL_RTX && ! CONSTANT_P (disp)) || (scale != NULL_RTX && ! CONSTANT_P (scale))) { - /* Its is not an address generation. Probably we have no 3 op + /* It is not an address generation. Probably we have no 3 op add. Last chance is to use 2-op add insn. */ lra_assert (x != y && x != z); - emit_move_insn (x, z); - insn = gen_add2_insn (x, y); + emit_move_insn (x, y); + insn = gen_add2_insn (x, z); emit_insn (insn); } else Index: testsuite/gcc.target/s390/pr57559.c =================================================================== --- testsuite/gcc.target/s390/pr57559.c (revision 0) +++ testsuite/gcc.target/s390/pr57559.c (working copy) @@ -0,0 +1,24 @@ +/* PR rtl-optimization/57559 */ + +/* { dg-do compile } */ +/* { dg-options "-march=z10 -m64 -mzarch -O1" } */ + +typedef int int32_t; +typedef unsigned char uint8_t; +typedef unsigned int uint32_t; +struct _IO_marker +{ +}; +static const int32_t mfcone = 1; +static const uint8_t *mfctop = (const uint8_t *) &mfcone; +int32_t +decContextTestEndian (uint8_t quiet) +{ + int32_t res = 0; + uint32_t dle = (uint32_t) 0; + if (*(int *) 10 != 0) + { + res = (int32_t) * mfctop - dle; + } + return res; +}
The following patch fixes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57559 The problem was in systematic discouraging reload symbol address in memory. The patch actually reverts a fix for a x86/x86-64 PR for -fpie. The patch implements another solution for this problem (see changes for lra_emit_add). The patch was successfully bootstrapped and tested on x86/x86-64 and s390. Committed as rev. 199859. 2013-06-08 Vladimir Makarov <vmakarov@redhat.com> PR rtl-optimization/57559 * lra-constraints.c (process_alt_operands): Don't discourage memory with known offset for offsetable memory constraint. * lra.c (lra_emit_add): Exchange y and z for 2-op add insn. 2013-06-08 Vladimir Makarov <vmakarov@redhat.com> PR rtl-optimization/57559 * gcc.target/s390/pr57559.c : New test.