This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix PR optimization/6475


Hi!

PR optimization/6475 is miscompiled with various options on IA-32.
What's going on is basically:
1) GCSE thinks it is a good idea to CSE (frame + const_int)
2) reg_scan_mark_refs assigns identical REGNO_DECL to the pseudo
   newly created by GCSE above (219) as to the original REG/v for the
   variable (78)
3) alter_reg is called with from_reg set to the same value for
   the pseudo 82 and pseudo 219, thus the same stack slot is
   reused for the two.
   Now we have:
   (mem:SI (plus:SI (reg/f:SI 6 ebp) (const_int -224 [0xffffff20])) [17 p S4 A8])
   (mem:SI (plus:SI (reg/f:SI 6 ebp) (const_int -272 [0xfffffef0])) [27 p S4 A8])
   (mem:SI (plus:SI (reg/f:SI 6 ebp) (const_int -272 [0xfffffef0])) [27 xx S4 A8])
4) when write_dependence_p is called during scheduling with the last
   two MEMs as arguments, it returns they don't conflict, because
   nonoverlapping_memrefs_p thinks so. This is because both have
   expressions with DECL_RTL set, but in one case it is -272(%ebp)
   and in the other case -224(%ebp). It compares them, sees the DECL_RTL
   don't conflicts and returns 1.
5) thus sched2 decides to swap the store into xx with load from p,
   thus overwriting what was in p before it is read and we segfault at
   runtime

I'm a little bit confused by alias.c comment
  /* If either RTL is not a MEM, it must be a REG or CONCAT, meaning they
     can't overlap unless they are the same because we never reuse that part
     of the stack frame used for locals for spilled pseudos.  */
because my understanding of what alter_reg does is exactly that it reuses
stack frame slots for pseudos to be spilled.

Anyway, below are two possible patches which seem to fix this testcase,
though I'm not sure if something completely else wouldn't be better
(I'm afraid if it won't cause performance regressions).
One patch only assigns exprs to MEMs if alter_reg parameter is the `home'
pseudo of REGNO_DECL (i), otherwise leaves them with alias/etc. memattrs
only, the other patch assigns exprs to all MEMs, but disables sharing
for such regnos.

What do you think?

	Jakub

Attachment: P
Description: Text document

Attachment: P2
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]