Improve DSE in the presence of calls

[Easwaran Raman]

On Tue, Jun 14, 2011 at 5:47 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, May 10, 2011 at 12:18 PM, Easwaran Raman <eraman@google.com> wrote:
>> On Tue, May 3, 2011 at 9:40 AM, Easwaran Raman <eraman@google.com> wrote:
>>> On Mon, May 2, 2011 at 8:37 PM, Jeff Law <law@redhat.com> wrote:
>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>> Hash: SHA1
>>>>
>>>> On 04/26/11 16:06, Easwaran Raman wrote:
>>>>
>>>>>
>>>>>> You're right. The patch has correctness issues. It is not possible to
>>>>>> simply not call add_wild_read because it also resets
>>>>>> active_local_stores and frees read_recs. During the local phase it
>>>>>> seems better to just treat calls as wild reads and reset
>>>>>> active_local_stores and free read_recs. I've now refactored
>>>>>> add_wild_read so that resetting active_local_stores and free read_recs
>>>>>> are in separate methods that can be called on non-const/non-memset
>>>>>> calls. In addition, I have added a non_frame_wild_read field in
>>>>>> insn_info to mark non-const and non-memset calls.
>>>>>
>>>>>> I've attached the revised patch.
>>>> Looking better.  Just a few more things.
>>>>
>>>> Don't all locals escape if the callee has a static chain?  Is that
>>>> handled anywhere?
>>>>
>>>> Don't you still have the potential for wild reads in dse_step5_nospill
>>>> (say from an asm)?  if so, then the change can't be correct.
>>>>
>>>> Couldn't you just add a clause like this before the else-if?
>>>>
>>>> else if (insn_info->non_frame_wild_read)
>>>>  {
>>>>    if (dump_file)
>>>>      fprintf (dump_file, "non-frame wild read\n");
>>>>    scan_reads_nospill (insn_info, v, NULL);
>>>>  }
>>>> else if (insn_info->read_rec)
>>>>  {
>>>>    /* Leave this clause unchanged */
>>>>  }
>>>>
>>>> Am I missing something?
>>>
>>> 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)
>>
>>
>
> One test failed on Linux/ia32:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49414
>
> --
> H.J.
>

Sorry about the ia32 test failure.  Looking into it now.

-Easwaran