[PATCH] D118558: [IVDescriptors] Support FOR where we have multiple sink pointed

Mon Feb 7 19:24:49 PST 2022

Allen marked an inline comment as done.
Allen added a comment.

@fhahn ping

================
Comment at: llvm/lib/Analysis/IVDescriptors.cpp:932
+        return true;
+    }

----------------
fhahn wrote:
> I think this handles the case where Previous doesn't come before LastPrev incorrectly. I added an additional test that crashes with the current version of the patch (02ee3fbff816).
> 
> To start with, you should be able to use `return Previous->comesBefore(LastPrev);` here. The reasoning is that if Previous comes before LastPrev, `SinkCandidate` already gets sunk after `Previous`, so there's nothing to do here.
> 
> I put up a follow-up patch (D118642), that should handle the other case correctly.
Yes, Thanks a lot for extending.

what need I do next step ? do you mind squash your patch D118642, or just fix the crash of your new added case 02ee3fbff816 ?

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118558/new/

https://reviews.llvm.org/D118558