[Bug rtl-optimization/78652] New: LRA generates wrong code by inheriting changed register

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] New: LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

            Bug ID: 78652
           Summary: LRA generates wrong code by inheriting changed
                    register
           Product: gcc
           Version: 7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: amker at gcc dot gnu.org
  Target Milestone: ---

Hi, given below sample code:

extern int printf(const char *, ...);

short a, b = 1, p, t;
struct {
  signed f1 : 9;
} m = {3};

int c, g = 0, h = 8, i, k, l, q, r, s, w, x = 9;
long long d = 1;
long long e = 1;
char f[9][6][4] = {{{1}}};
short j[6] = {1};

unsigned n;
static long long *o = &e;
char u;
short *v;
char *y = &f[6][4][3];
short fn1(short p1) { return a == 0 ? p1 : p1 % a; }

static int *fn2(signed char p1, int p2, signed char p3) {
lbl_2057:
  if (n >= (q >= fn1(3)))
    return &r;
  if (p2)
    t = u;
  *v = (t | *o) * p2;
  for (;;) {
    for (; u <= 1;)
      goto lbl_2057;
    if (p1)
      s = g = 0;
  }
}

void fn3(int *p1) {
  if (*p1)
    ;
  else {
    x = w;
    for (;;)
      ;
  }
}

int *fn4(long p1) {
  for (; p1; p1--)
    e = 1;
  y = 0;
  return &h;
}

__attribute__((section(".text")))
int main() {
  int *z;
  long t1;
  long long *t2 = &d;
  *t2 = b;
  z = fn4(*t2);
  fn3(z);
  short *t3 = &j[0];
  *t3 = f[6][4][3] >= (b = i);
  t1 = p < n;
  fn2(c < t1 >= m.f1, l, k);
  j[5]++;
  printf("result = %X\n", m.f1);
}

Compiled/run with below command line on arm-none-eabi:
$ arm-none-eabi-gcc -march=armv7-a -fno-strict-aliasing test.c -o test.exe
-specs=aprofile-validation.specs -O2
$ ./test.exe
result = 1

Which should be:
result = 3

This doesn't happen with O1.
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

--- Comment #1 from amker at gcc dot gnu.org ---
GCC is configured as below:
--target=arm-none-eabi --prefix=... --with-gmp=... --with-mpfr=...
--with-mpc=... --with-isl=... --disable-shared --disable-nls --disable-threads
--disable-tls --enable-checking=yes --enable-languages=c,c++,fortran
--with-newlib

Piece of generated assmebly is as:
        ldr     r5, .L44        <------init r5
        ldrsh   r0, [r0]        
        movt    r3, #:upper16:l
        movcs   r1, #0  
        movcc   r1, #1  
        cmp     lr, r1  
        ldrh    ip, [r5, #12]!  <------modify r5
        ldr     r3, [r3]        
        movw    fp, #:lower16:v
        movge   r1, #0  
        movlt   r1, #1  
        str     r1, [sp, #8]    
        movt    fp, #:upper16:v
        ldrb    r1, [r4, #203]  
        movw    r2, #:lower16:a
        movw    r9, #:lower16:u
        movw    r8, #:lower16:t
        cmp     r1, r0  
        movw    r7, #:lower16:.LANCHOR1
        movw    r6, #:lower16:s
        movt    r2, #:upper16:a
        movlt   r1, #0  
        movge   r1, #1  
        movt    r9, #:upper16:u
        strh    r1, [r5]        <------inherit use of changed r5.

It's clear LRA inherits register with changed value for r5.  Looking at below
dump of IRA:
   51: r262:SI=const(`*.LANCHOR0'+0x100)       <------init r262
      REG_EQUIV const(`*.LANCHOR0'+0x100)
  277: r186:SI=high(`p')
      REG_EQUAL high(`p')
  279: r258:SI=high(`n')
      REG_EQUAL high(`n')
  278: r186:SI=r186:SI+low(`p')
      REG_EQUAL `p'
  280: r258:SI=r258:SI+low(`n')
      REG_EQUAL `n'
  275: r185:SI=high(`c')
      REG_EQUAL high(`c')
  276: r185:SI=r185:SI+low(`c')
      REG_EQUAL `c'
   58: r187:SI=sign_extend([r186:SI])
      REG_DEAD r186:SI
      REG_EQUAL sign_extend([`p'])
   60: r190:SI=[r258:SI]
      REG_EQUIV [r258:SI]
      REG_EQUAL [`n']
  273: r176:SI=high(`i')
      REG_EQUAL high(`i')
  254: r263:SI=r262:SI                          <------init r263 with r262
  274: r176:SI=r176:SI+low(`i')
      REG_EQUAL `i'
  281: r203:SI=high(`l')
      REG_EQUAL high(`l')
  282: r203:SI=r203:SI+low(`l')
      REG_EQUAL `l'
   63: r193:SI=[r185:SI]
      REG_DEAD r185:SI
      REG_EQUAL [`c']
   62: {r191:SI=ltu(r187:SI,r190:SI);clobber cc:CC;}
      REG_DEAD r190:SI
      REG_DEAD r187:SI
      REG_UNUSED cc:CC
   71: r201:SI=zero_extend([pre r263:SI+=0xc])  <------r263 is modified
      REG_INC r263:SI
      REG_EQUAL zero_extend([const(`*.LANCHOR0'+0x10c)])
  283: r266:SI=high(`a')
      REG_EQUAL high(`a')
   45: r116:SI=sign_extend([r176:SI])
      REG_DEAD r176:SI
   65: {r126:SI=r193:SI<r191:SI;clobber cc:CC;}
      REG_DEAD r193:SI
      REG_DEAD r191:SI
      REG_UNUSED cc:CC
   76: r130:SI=[r203:SI]
      REG_DEAD r203:SI
      REG_EQUAL [`l']
   74: r128:SI=sign_extract(r201:SI,0x9,0)
      REG_DEAD r201:SI
   42: r114:SI=zero_extend([r264:SI+0xcb])
      REG_EQUAL zero_extend([const(`*.LANCHOR0'+0xcb)])
  284: r266:SI=r266:SI+low(`a')
      REG_EQUAL `a'
  285: r259:SI=high(`q')
      REG_EQUAL high(`q')
  287: r272:SI=high(`u')
      REG_EQUAL high(`u')
  286: r259:SI=r259:SI+low(`q')
      REG_EQUAL `q'
  288: r272:SI=r272:SI+low(`u')
      REG_EQUAL `u'
  289: r273:SI=high(`t')
      REG_EQUAL high(`t')
  291: r274:SI=high(`v')
      REG_EQUAL high(`v')
  290: r273:SI=r273:SI+low(`t')
      REG_EQUAL `t'
  292: r274:SI=r274:SI+low(`v')
      REG_EQUAL `v'
  293: r275:SI=high(`*.LANCHOR1')
      REG_EQUAL high(`*.LANCHOR1')
  294: r275:SI=r275:SI+low(`*.LANCHOR1')
      REG_EQUAL `*.LANCHOR1'
  295: r276:SI=high(`s')
      REG_EQUAL high(`s')
  296: r276:SI=r276:SI+low(`s')
      REG_EQUAL `s'
   47: [r264:SI+0x18]=r116:SI#0
   53: {r184:SI=r114:SI>=r116:SI;clobber cc:CC;}
      REG_DEAD r116:SI
      REG_DEAD r114:SI
      REG_UNUSED cc:CC
   55: [r262:SI]=r184:SI#0                       <-------use of r262

It results in assigning the same hard register (r5) to both r263 and r262.
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

amker at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |6.2.1
      Known to fail|                            |7.0

--- Comment #2 from amker at gcc dot gnu.org ---
Tested on 6.2.1, it works.
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] [7 regression]LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

--- Comment #3 from amker at gcc dot gnu.org ---
I bisected and found it was caused by below change:

commit ab4ea053bf71c5571df344d5d5f5bd7ecc5ede8e
Author: vmakarov <vmakarov@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Tue Aug 2 16:07:36 2016 +0000

    2016-08-02  Vladimir Makarov  <[hidden email]>

        PR rtl-optimization/69847
        * lra-int.h (struct lra-reg): Use restore_rtx instead of
        restore_regno.
        (lra_rtx_hash): New.
        * lra.c (initialize_lra_reg_info_element): Use restore_rtx instead
        of restore_regno.
        (lra_rtx_hash): Rename and move lra-remat.c::rtx_hash.
        * lra-remat.c (rtx_hash): Rename and Move to lra.c.
        * lra-spills.c (lra_final_code_change): Don't delete insn when the
        next insn is USE with the same reg as the current insn source.
        * lra-constraints.c (curr_insn_transform): Use restore_rtx instead
        of restore_regno.
        (lra_constraints_init): Call initiate_invariants.
        (lra_constraints_finish): Call finish_invariants.
        (struct invariant, invariant_t, invariant_ptr_t): New.
        (const_invariant_ptr_t, invariants, invariants_pool): New.
        (invariant_table, invariant_hash, invariant_eq_p): New.
        (insert_invariant, initiate_invariants, finish_invariants): New.
        (clear_invariants, invalid_invariant_regs): New.
        (inherit_reload_reg, split_reg, fix_bb_live_info): Use restore_rtx
        instead of restore_regno.
        (invariant_p, process_invariant_for_inheritance): New.
        (inherit_in_ebb): Implement invariant inheritance.
        (lra_inheritance): Initialize and finalize invalid_invariant_regs.
        (remove_inheritance_pseudos): Implement undoing invariant
        inheritance.
        (undo_optional_reloads, lra_undo_inheritance): Use restore_rtx
        instead of restore_regno.
        * lra-assigns.c (regno_live_length): New.
        (reload_pseudo_compare_func): Use regno_live_length.
        (assign_by_spills): Use restore_rtx instead of restore_regno.
        (lra_assign): Ditto.  Initiate regno_live_length.



    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@238991
138bc75d-0d04-0410-961f-82ee72b054a4

It is a big change.
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] [7 regression]LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

--- Comment #4 from amker at gcc dot gnu.org ---
Looks like Vlad already fixed this @ below commit:

commit 6526e1b66785b76d71926b20c9eb299e74a2d255
Author: vmakarov <vmakarov@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed Nov 30 17:35:40 2016 +0000

    2016-11-30  Vladimir Makarov  <[hidden email]>

        PR tree-optimization/77856
        * lra-constraints.c (inherit_in_ebb): Check original regno for
        invalid invariant regs too.  Set only clobbered hard regs for the
        invalid invariant regs.

    2016-11-30  Vladimir Makarov  <[hidden email]>

        PR tree-optimization/77856
        * gcc.target/i386.c (pr77856.c): New.



    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@243038
138bc75d-0d04-0410-961f-82ee72b054a4


I will double check and close this PR.

Thanks.
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] [7 regression]LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

--- Comment #5 from amker at gcc dot gnu.org ---
Indeed, it was fixed by that.  I will a test for this and then close.
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] [7 regression]LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |7.0
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] Add testcase for LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2017-01-05
                 CC|                            |law at redhat dot com
            Summary|[7 regression]LRA generates |Add testcase for LRA
                   |wrong code by inheriting    |generates wrong code by
                   |changed register            |inheriting changed register
     Ever confirmed|0                           |1

--- Comment #6 from Jeffrey A. Law <law at redhat dot com> ---
No longer a regression, just a need to add a testcase.
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] Add testcase for LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|7.0                         |7.2

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 7.1 has been released.
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] Add testcase for LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|7.2                         |7.3

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 7.2 is being released, adjusting target milestone.

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 7.2 is being released, adjusting target milestone.
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] Add testcase for LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|7.2                         |7.3

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 7.2 is being released, adjusting target milestone.

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 7.2 is being released, adjusting target milestone.
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] Add testcase for LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|7.2                         |7.3
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] Add testcase for LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|7.3                         |7.4

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 7.3 is being released, adjusting target milestone.
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] Add testcase for LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|7.4                         |7.5
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/78652] Add testcase for LRA generates wrong code by inheriting changed register

abensonca at gmail dot com
In reply to this post by abensonca at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78652

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|7.5                         |---