This patch seems to break ia64 and some other targets. I have updated
Bug 49429 with a test case triage. It looks like for some EXPR,
may_be_aliased returns incorrect value and hence can_escape
incorrectly concludes the variable can't escape resulting in removal
of useful stores.
(So can_escape() returns false. But later on, in the same BB, I see:
In the following list of instructions,
(insn 4 3 6 2 (set (mem/s/c:DI (reg/f:DI 341) [2 s1+0 S8 A64])
(reg:DI 112 in0)) y.c:23 5 {movdi_internal}
(expr_list:REG_DEAD (reg:DI 112 in0)
(nil)))
...
(insn 36 30 37 2 (set (reg:DI 120 out0)
(reg/f:DI 357)) 5 {movdi_internal}
(expr_list:REG_EQUAL (plus:DI (reg/f:DI 328 sfp)
(const_int 62 [0x3e]))
(nil)))
(insn 37 36 38 2 (set (reg:DI 121 out1)
(reg/f:DI 341)) 5 {movdi_internal}
(expr_list:REG_DEAD (reg/f:DI 341)
(expr_list:REG_EQUAL (plus:DI (reg/f:DI 328 sfp)
(const_int 96 [0x60]))
(nil))))
(insn 38 37 39 2 (set (reg:DI 122 out2)
(const_int 31 [0x1f])) 5 {movdi_internal}
(nil))
(call_insn 39 38 42 2 (parallel [
(set (reg:DI 8 r8)
(call (mem:DI (symbol_ref:DI ("memcpy") [flags 0x41]
<function_decl 0x7ffff70d2e00 memcpy>) [0 memcpy S8 A64])
(const_int 1 [0x1])))
(clobber (reg:DI 320 b0))
(clobber (scratch:DI))
(clobber (scratch:DI))
]) 332 {call_value_gp}
(expr_list:REG_DEAD (reg:DI 122 out2)
(expr_list:REG_DEAD (reg:DI 121 out1)
(expr_list:REG_DEAD (reg:DI 120 out0)
(expr_list:REG_UNUSED (reg:DI 8 r8)
(expr_list:REG_EH_REGION (const_int 0 [0])
(nil))))))
(expr_list:REG_DEP_TRUE (use (reg:DI 1 r1))
(expr_list:REG_DEP_TRUE (use (reg:DI 122 out2))
(expr_list:REG_DEP_TRUE (use (reg:DI 121 out1))
(expr_list:REG_DEP_TRUE (use (reg:DI 120 out0))
(nil))))))
for the memory expression
(set (mem/s/c:DI (reg/f:DI 341) [2 s1+0 S8 A64])
(reg:DI 112 in0))
may_be_aliased() returns false, but register 341 is passed as the
second parameter to memcpy. I suspect this is a bug elsewhere which is
exposed by this patch. If someone knows why this might be happening, I
can tighten the can_escape() function appropriately.
Thanks,
Easwaran
On Tue, Jun 14, 2011 at 9:34 AM, Jeff Law <l...@redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 05/10/11 13:18, Easwaran Raman wrote:
>
>>>> I am not sure I understand the problem here. If there is a wild read
>>>> from asm, the instruction has the wild_read flag set. The if statement
>>>> checks if that flag is set and if so it clears the bitmap - which was
>>>> the original behavior. Originally, only if read_rec is non NULL you
>>>> need to recompute the kill set. Now, even if read_rec is NULL,
>>>> non_frame_wild_read could be set requiring the kill set to be
>>>> modified, which is what this patch does. In fact, isn't what you have
>>>> written above the equivalent to what is in the patch as '/* Leave this
>>>> clause unchanged */' is the same as
>>>>
>>>> if (dump_file)
>>>> fprintf (dump_file, "regular read\n");
>>>> scan_reads_nospill (insn_info, v, NULL);
>>>>
>>>>
>>>> -Easwaran
>>>>
>>
>>> Ping. I have changed the test case to use int and added another test
>>> case that shows DSE doesn't happen when the struct instance is
>>> volatile (wild_read gets set in that case)
>>
>>
>> What's the purpose behind using unit64_t in the testcase? Somehow I
>> suspect using int64_t means the test is unlikely not going to work
>> across targets with different word sizes.
> Sorry for the exceedingly long wait. Things have been a bit crazy the
> last several weeks.
>
> On a positive note, re-reading things now I think my objection/comment
> was mis-guided.
>
> Patch approved, and again, sorry for the absurdly long period of
> non-responsiveness.
>
> jeff
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iQEcBAEBAgAGBQJN942IAAoJEBRtltQi2kC7Fj4IAIUvXsEKHZEKHS2k/psJWyaM
> Uo/vW3CLydRP0+Np/VVSwzHlmWfdWmOj1WPw1Svhvr4gP8BrZ13okVv5jbw1Hh3Y
> R4mShXFK5eYzmGx5wL54hOze5zViN3gomNGbDAAhk6TCzNXmPyLT/6V1tLFTNhD5
> 6zOiW8pXh9ik6qTTCKbG0EMuJXDnIbYrJs4d/gHFerUgmRPc8adKjF3PCngD3F4r
> 40n9W/UxUejYUddavDW1fIdALWYc56F3glplFsII7SMnOmih8MTFYOvk6SZsLS5O
> G2nzmnUuwt6tPWTyk9bpVKQi5dn8MmLkM13w22t36GKIg6OER2KfUdv44dgE7yw=
> =o7AI
> -----END PGP SIGNATURE-----
>
