Improve DSE in the presence of calls

Easwaran Raman eraman@google.com
Fri Jun 17 22:44:00 GMT 2011


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 <law@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-----
>



More information about the Gcc-patches mailing list