| Message ID | 54933E72.6070702@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Dec 18, 2014 at 12:52 PM, Vladimir Makarov <vmakarov@redhat.com> wrote: > The following patch solves > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64291 > > It is a bug in a new rematerialization subpass of LRA. > > The patch was bootstrapped on x86/x86-64. > > Committed as rev. 218874. > > 2014-12-18 Vladimir Makarov <vmakarov@redhat.com> > > PR rtl-optimization/64291 > * lra-remat.c (bad_for_rematerialization_p): Add UNPSEC_VLOATILE. > (create_cands): Process only output reload insn with potential > cands. > > 2014-12-18 Vladimir Makarov <vmakarov@redhat.com> > > PR rtl-optimization/64291 > * testsuite/gcc.target/i386/pr64291-[12].c: New tests. > There are a couple problems with the testcase: 1. It has typedef struct { int _mp_size; unsigned long *_mp_d; } __mpz_struct; typedef __mpz_struct mpz_t[1]; int main () { mpz_t n, d; long nn, dn; unsigned long *np, *dup, *dnp, *qp; long alloc, itch; f (n); f (d); qp = (unsigned long*)__builtin_alloca(4099*8) + 1; dnp = (unsigned long*)__builtin_alloca (2049*8); alloc = 1; for (test = 0; test < 1; test++) { dn = d->_mp_size; dup = d->_mp_d; f (dnp, dup, dn); dnp[dn - 1] |= 1UL<<63; For 32-bit targets, like -m32 and -mx32, GCC complains: /export/gnu/import/git/gcc/gcc/testsuite/gcc.target/i386/pr64291-1.c:33:25: warning: left shift count >= width of type [-Wshift-count-overflow] 2. This program uses uninitialized stack variable n, d: void f(void*p,...){} The behavior of this testcase is totally undefined.
On Sun, Dec 28, 2014 at 7:28 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Dec 18, 2014 at 12:52 PM, Vladimir Makarov <vmakarov@redhat.com> wrote: >> The following patch solves >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64291 >> >> It is a bug in a new rematerialization subpass of LRA. >> >> The patch was bootstrapped on x86/x86-64. >> >> Committed as rev. 218874. >> >> 2014-12-18 Vladimir Makarov <vmakarov@redhat.com> >> >> PR rtl-optimization/64291 >> * lra-remat.c (bad_for_rematerialization_p): Add UNPSEC_VLOATILE. >> (create_cands): Process only output reload insn with potential >> cands. >> >> 2014-12-18 Vladimir Makarov <vmakarov@redhat.com> >> >> PR rtl-optimization/64291 >> * testsuite/gcc.target/i386/pr64291-[12].c: New tests. >> > > There are a couple problems with the testcase: > > 1. It has > > typedef struct > { > int _mp_size; > unsigned long *_mp_d; > } __mpz_struct; > typedef __mpz_struct mpz_t[1]; > > int main () > { > mpz_t n, d; > long nn, dn; > unsigned long *np, *dup, *dnp, *qp; > long alloc, itch; > > f (n); > f (d); > qp = (unsigned long*)__builtin_alloca(4099*8) + 1; > dnp = (unsigned long*)__builtin_alloca (2049*8); > alloc = 1; > for (test = 0; test < 1; test++) > { > dn = d->_mp_size; > dup = d->_mp_d; > f (dnp, dup, dn); > dnp[dn - 1] |= 1UL<<63; > > For 32-bit targets, like -m32 and -mx32, GCC complains: > > /export/gnu/import/git/gcc/gcc/testsuite/gcc.target/i386/pr64291-1.c:33:25: > warning: left shift count >= width of type [-Wshift-count-overflow] > > 2. This program uses uninitialized stack variable n, d: > > void f(void*p,...){} > > The behavior of this testcase is totally undefined. > I opened: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64291
On Sun, Dec 28, 2014 at 7:34 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Sun, Dec 28, 2014 at 7:28 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Dec 18, 2014 at 12:52 PM, Vladimir Makarov <vmakarov@redhat.com> wrote: >>> The following patch solves >>> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64291 >>> >>> It is a bug in a new rematerialization subpass of LRA. >>> >>> The patch was bootstrapped on x86/x86-64. >>> >>> Committed as rev. 218874. >>> >>> 2014-12-18 Vladimir Makarov <vmakarov@redhat.com> >>> >>> PR rtl-optimization/64291 >>> * lra-remat.c (bad_for_rematerialization_p): Add UNPSEC_VLOATILE. >>> (create_cands): Process only output reload insn with potential >>> cands. >>> >>> 2014-12-18 Vladimir Makarov <vmakarov@redhat.com> >>> >>> PR rtl-optimization/64291 >>> * testsuite/gcc.target/i386/pr64291-[12].c: New tests. >>> >> >> There are a couple problems with the testcase: >> >> 1. It has >> >> typedef struct >> { >> int _mp_size; >> unsigned long *_mp_d; >> } __mpz_struct; >> typedef __mpz_struct mpz_t[1]; >> >> int main () >> { >> mpz_t n, d; >> long nn, dn; >> unsigned long *np, *dup, *dnp, *qp; >> long alloc, itch; >> >> f (n); >> f (d); >> qp = (unsigned long*)__builtin_alloca(4099*8) + 1; >> dnp = (unsigned long*)__builtin_alloca (2049*8); >> alloc = 1; >> for (test = 0; test < 1; test++) >> { >> dn = d->_mp_size; >> dup = d->_mp_d; >> f (dnp, dup, dn); >> dnp[dn - 1] |= 1UL<<63; >> >> For 32-bit targets, like -m32 and -mx32, GCC complains: >> >> /export/gnu/import/git/gcc/gcc/testsuite/gcc.target/i386/pr64291-1.c:33:25: >> warning: left shift count >= width of type [-Wshift-count-overflow] >> >> 2. This program uses uninitialized stack variable n, d: >> >> void f(void*p,...){} >> >> The behavior of this testcase is totally undefined. >> > > I opened: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64291 > Oops. I meant: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64427
Index: lra-remat.c =================================================================== --- lra-remat.c (revision 218685) +++ lra-remat.c (working copy) @@ -350,12 +350,12 @@ finish_cand_table (void) -/* Return true if X contains memory or UNSPEC. We can not just check - insn operands as memory or unspec might be not an operand itself - but contain an operand. Insn with memory access is not profitable - for rematerialization. Rematerialization of UNSPEC might result in - wrong code generation as the UNPEC effect is unknown - (e.g. generating a label). */ +/* Return true if X contains memory or some UNSPEC. We can not just + check insn operands as memory or unspec might be not an operand + itself but contain an operand. Insn with memory access is not + profitable for rematerialization. Rematerialization of UNSPEC + might result in wrong code generation as the UNPEC effect is + unknown (e.g. generating a label). */ static bool bad_for_rematerialization_p (rtx x) { @@ -363,7 +363,7 @@ bad_for_rematerialization_p (rtx x) const char *fmt; enum rtx_code code; - if (MEM_P (x) || GET_CODE (x) == UNSPEC) + if (MEM_P (x) || GET_CODE (x) == UNSPEC || GET_CODE (x) == UNSPEC_VOLATILE) return true; code = GET_CODE (x); fmt = GET_RTX_FORMAT (code); @@ -406,7 +406,7 @@ operand_to_remat (rtx_insn *insn) if (reg->regno == STACK_POINTER_REGNUM && frame_pointer_needed) return -1; else if (reg->type == OP_OUT && ! reg->subreg_p - && find_regno_note (insn, REG_UNUSED, reg->regno) == NULL) + && find_regno_note (insn, REG_UNUSED, reg->regno) == NULL) { /* We permits only one spilled reg. */ if (found_reg != NULL) @@ -508,11 +508,14 @@ create_cands (void) if ((set = single_set (insn)) != NULL && REG_P (SET_SRC (set)) && REG_P (SET_DEST (set)) - && (src_regno = REGNO (SET_SRC (set))) >= FIRST_PSEUDO_REGISTER + && ((src_regno = REGNO (SET_SRC (set))) + >= lra_constraint_new_regno_start) && (dst_regno = REGNO (SET_DEST (set))) >= FIRST_PSEUDO_REGISTER && reg_renumber[dst_regno] < 0 && (insn2 = regno_potential_cand[src_regno].insn) != NULL && BLOCK_FOR_INSN (insn2) == BLOCK_FOR_INSN (insn)) + /* It is an output reload insn after insn can be + rematerialized (potential candidate). */ create_cand (insn2, regno_potential_cand[src_regno].nop, dst_regno); if (nop < 0) goto fail; Index: testsuite/gcc.target/i386/pr64291-1.c =================================================================== --- testsuite/gcc.target/i386/pr64291-1.c (revision 0) +++ testsuite/gcc.target/i386/pr64291-1.c (working copy) @@ -0,0 +1,51 @@ +/* { dg-options "-O2" } */ +/* { dg-additional-sources pr64291-2.c } */ +/* { dg-do run } */ +void f(void*,...); +void g(void*,long,long); +int nnn=0; +long test=0; + +typedef struct +{ + int _mp_size; + unsigned long *_mp_d; +} __mpz_struct; +typedef __mpz_struct mpz_t[1]; + +int main () +{ + mpz_t n, d; + long nn, dn; + unsigned long *np, *dup, *dnp, *qp; + long alloc, itch; + + f (n); + f (d); + qp = (unsigned long*)__builtin_alloca(4099*8) + 1; + dnp = (unsigned long*)__builtin_alloca (2049*8); + alloc = 1; + for (test = 0; test < 1; test++) + { + dn = d->_mp_size; + dup = d->_mp_d; + f (dnp, dup, dn); + dnp[dn - 1] |= 1UL<<63; + f (0); + nn = nnn; + np = n->_mp_d; + qp[-1] = -757136820; + qp[nn - dn + 1] = 14883681; + f (0); + if (dn >= 6) + f (0); + itch = nn + 1; + if (itch + 1> alloc) + { + g(0,alloc*8,(itch+1)*8); + alloc = itch + 1; + } + f (np, nn); + } + return 0; +} Index: testsuite/gcc.target/i386/pr64291-2.c =================================================================== --- testsuite/gcc.target/i386/pr64291-2.c (revision 0) +++ testsuite/gcc.target/i386/pr64291-2.c (working copy) @@ -0,0 +1,4 @@ +/* { dg-do compile } */ +extern void abort (void); +void f(void*p,...){} +void g(void*p,long a,long b){if (a!=8) abort();}
The following patch solves https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64291 It is a bug in a new rematerialization subpass of LRA. The patch was bootstrapped on x86/x86-64. Committed as rev. 218874. 2014-12-18 Vladimir Makarov <vmakarov@redhat.com> PR rtl-optimization/64291 * lra-remat.c (bad_for_rematerialization_p): Add UNPSEC_VLOATILE. (create_cands): Process only output reload insn with potential cands. 2014-12-18 Vladimir Makarov <vmakarov@redhat.com> PR rtl-optimization/64291 * testsuite/gcc.target/i386/pr64291-[12].c: New tests.