llvm dev - Sep 2017 - Potential infinite loop in MemorySSAUpdater

Hi,

Can some one explain the intended behaviour of following loop in void
MemorySSAUpdater::insertDef(MemoryDef *MD, bool RenameUses) function.

  while (!FixupList.empty()) {
    unsigned StartingPHISize = InsertedPHIs.size();
    fixupDefs(FixupList);
    FixupList.clear();
    // Put any new phis on the fixup list, and process them
    FixupList.append(InsertedPHIs.end() - StartingPHISize, InsertedPHIs.end());
  }

With the latest code on trunk compilation of perlbench SPEC CPU 2017 INT
benchmark with “-O3 -inline-threshold=1000 and -enable-gvn-hoist” options is
looping infinitely on the above loop.

Above loop never terminates unless elements from InsertedPHIs are removed as and
when they are processed. But, I don’t see any call to remove contents, delete or
clear, from InsertedPHIs.

With regards
Bhargav Reddy Godala
Software Engineer 2
Bangalore, India
E-mail: Bhargav-reddy.Godala at amd.com<mailto:Bhargav-reddy.Godala at
amd.com> Ext 30678



-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170923/18eae61e/attachment.html>

On Sat, Sep 23, 2017 at 8:38 AM, Godala, Bhargav-reddy via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi,
>
> Can some one explain the intended behaviour of following loop in void
> MemorySSAUpdater::insertDef(MemoryDef *MD, bool RenameUses) function.
>
>   while (!FixupList.empty()) {
>     unsigned StartingPHISize = InsertedPHIs.size();
>     fixupDefs(FixupList);
>     FixupList.clear();
>     // Put any new phis on the fixup list, and process them
>     FixupList.append(InsertedPHIs.end() - StartingPHISize,
> InsertedPHIs.end());
>   }
>
> With the latest code on trunk compilation of perlbench SPEC CPU 2017 INT
> benchmark with “-O3 -inline-threshold=1000 and -enable-gvn-hoist” options
> is looping infinitely on the above loop.
>

>
> Above loop never terminates unless elements from InsertedPHIs are removed
> as and when they are processed.
>
Yes, the loop is slightly off.

The intention is to process any new phis added by fixupdefs.
However, it really should be InsertedPHIs.start() + StartingPHISize.
That will insert only the phis that fixupdefs added, and it will terminate
as soon as fixupdefs stops adding phis.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170923/e3b53659/attachment.html>

With regards
Bhargav Reddy Godala
Software Engineer 2
Bangalore, India
E-mail: Bhargav-reddy.Godala at amd.com<mailto:Bhargav-reddy.Godala at
amd.com> Ext 30678



On 23-Sep-2017, at 9:27 PM, Daniel Berlin <dberlin at
dberlin.org<mailto:dberlin at dberlin.org>> wrote:



On Sat, Sep 23, 2017 at 8:38 AM, Godala, Bhargav-reddy via llvm-dev <llvm-dev
at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:
Hi,

Can some one explain the intended behaviour of following loop in void
MemorySSAUpdater::insertDef(MemoryDef *MD, bool RenameUses) function.

  while (!FixupList.empty()) {
    unsigned StartingPHISize = InsertedPHIs.size();
    fixupDefs(FixupList);
    FixupList.clear();
    // Put any new phis on the fixup list, and process them
    FixupList.append(InsertedPHIs.end() - StartingPHISize, InsertedPHIs.end());
  }

With the latest code on trunk compilation of perlbench SPEC CPU 2017 INT
benchmark with “-O3 -inline-threshold=1000 and -enable-gvn-hoist” options is
looping infinitely on the above loop.



Above loop never terminates unless elements from InsertedPHIs are removed as and
when they are processed.

Yes, the loop is slightly off.

The intention is to process any new phis added by fixupdefs.
However, it really should be InsertedPHIs.start() + StartingPHISize.

Even in that case it still is infinite, as there is no call to clear already
processed elements(MemoryPhi) in InsertedPHIs. fixupDefs function only inserts
new elements but not remove any that are processed.
That will insert only the phis that fixupdefs added, and it will terminate as
soon as fixupdefs stops adding phis.


-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170923/99f70e29/attachment-0001.html>