Bug 63906 - [5 Regression] lra_remat miscompiles glibc on aarch64
Summary: [5 Regression] lra_remat miscompiles glibc on aarch64
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 5.0
: P1 blocker
Target Milestone: 5.0
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2014-11-17 03:22 UTC by Andrew Pinski
Modified: 2014-11-18 03:23 UTC (History)
3 users (show)

See Also:
Host:
Target: aarch64-linux-gnu
Build:
Known to work:
Known to fail:
Last reconfirmed:

Attachments
the .i file (48.96 KB, text/plain)
2014-11-17 04:47 UTC, Andrew Pinski 		Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Pinski 2014-11-17 03:22:27 UTC 
I don't have much information yet but I wanted to file this so I don't lose track of it.

With lra_remat turned on, we get a crash inside dl_main.

   0x000000400080386c <dl_main+5604>:   ldr     x2, [x3,w0,uxtw #3]
=> 0x0000004000803870 <dl_main+5608>:   ldrb    w1, [x2,#788]
(gdb) p $x2
$1 = 0
Comment 1 Andrew Pinski 2014-11-17 03:52:59 UTC 
dl-deps.c is being miscompiled.
Comment 2 Andrew Pinski 2014-11-17 03:54:28 UTC 
(In reply to Andrew Pinski from comment #1)
> dl-deps.c is being miscompiled.

Specifically _dl_map_object_deps.
Comment 3 Andrew Pinski 2014-11-17 04:45:56 UTC 
It looks like dse is removing more stores when lra_remat is enable:

(insn 538 537 2663 41 (set (mem/f:DI (plus:DI (reg/f:DI 1 x1 [581])
                (const_int 16 [0x10])) [1 runp_409->next+0 S8 A128])
        (const_int 0 [0])) dl-deps.c:283 42 {*movdi_aarch64}
     (nil))


That is for:
		    newp->next = NULL;
		    tail->next = newp;

DSE (after reload) thinks tail->next is the same as the store to newp->next with LRA_REMAT turned on.
**scanning insn=538
  mem: (plus:DI (reg/f:DI 1 x1 [581])
    (const_int 16 [0x10]))

   after canon_rtx address: (plus:DI (reg/f:DI 1 x1 [581])
    (const_int 16 [0x10]))

   after cselib_expand address: (plus:DI (reg/f:DI 31 sp)
    (const_int 16 [0x10]))
....
**scanning insn=539
  mem: (plus:DI (reg/v/f:DI 2 x2 [orig:88 runp ] [88])
    (const_int 16 [0x10]))

   after canon_rtx address: (plus:DI (reg/v/f:DI 2 x2 [orig:88 runp ] [88])
    (const_int 16 [0x10]))

   after cselib_expand address: (plus:DI (reg/f:DI 31 sp)
    (const_int 16 [0x10]))

   after canon_rtx address: (plus:DI (reg/f:DI 31 sp)
    (const_int 16 [0x10]))
  varying cselib base=2:4287 offset = 16
 processing cselib store [16..24)
**scanning insn=539
  mem: (plus:DI (reg/v/f:DI 2 x2 [orig:88 runp ] [88])
    (const_int 16 [0x10]))

   after canon_rtx address: (plus:DI (reg/v/f:DI 2 x2 [orig:88 runp ] [88])
    (const_int 16 [0x10]))

   after cselib_expand address: (plus:DI (reg/f:DI 31 sp)
    (const_int 16 [0x10]))

   after canon_rtx address: (plus:DI (reg/f:DI 31 sp)
    (const_int 16 [0x10]))
  varying cselib base=2:4287 offset = 16
 processing cselib store [16..24)
Comment 4 Andrew Pinski 2014-11-17 04:47:29 UTC 
Created attachment 33996 [details]
the .i file
Comment 5 Andrew Pinski 2014-11-17 05:01:04 UTC 
No the problem is LRA_REMAT thinks:
(insn 2653 217 219 13 (set (reg/v/f:DI 3 x3 [orig:97 needed ] [97])
        (mem/c:DI (plus:DI (reg/f:DI 29 x29)
                (const_int 152 [0x98])) [34 %sfp+-136 S8 A64])) dl-deps.c:224 42 {*movdi_aarch64}
     (nil))

Is really sp but it is not as sp has changed just a few instructions above:
(insn 210 2652 214 13 (set (reg/f:DI 31 sp)
        (minus:DI (reg/f:DI 3 x3 [1110])
            (reg:DI 1 x1 [485]))) dl-deps.c:224 218 {subdi3}
     (nil))

Meaning LRA_remat cannot think sp is a constant if alloca is ever used.
Comment 6 Vladimir Makarov 2014-11-17 20:30:21 UTC 
(In reply to Andrew Pinski from comment #4)
> Created attachment 33996 [details]
> the .i file

Andrew, I'll start to work on this soon.  Sorry for inconvenience introduced by LRA remat. pass.
Comment 7 Vladimir Makarov 2014-11-18 00:14:57 UTC 
Author: vmakarov
Date: Tue Nov 18 00:14:25 2014
New Revision: 217683

URL: https://gcc.gnu.org/viewcvs?rev=217683&root=gcc&view=rev
Log:
2014-11-17  Vladimir Makarov  <vmakarov@redhat.com>

	PR rtl-optimization/63906
	* lra-remat.c (operand_to_remat): Check SP and
	frame_pointer_required.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/lra-remat.c
Comment 8 Andrew Pinski 2014-11-18 03:23:35 UTC 
Confirmed fixed.