[PATCH] D103016: NewGVN: Relax assertion about number of times a value is seen

Alina Sbirlea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 4 15:36:17 PDT 2021


asbirlea added a comment.

The issue in PR31613 is a legitimate bug, since two instructions that are not congruent end up in the same congruence class based on the PredicateInfo data. Increasing the limit will not help there since the exhibited behavior in much larger apps is "out of memory", the testcase I provided is only a reduced version triggering a debuggable assertion.

I haven't looked in detail into the testcase given in this patch yet. I can believe it's possible to legitimately get to that 100 threshold and that a bigger one may be needed. I can also believe it may be dependent on the number of blocks.
However I need to more thoroughly understand the testcase and how NewGVN gets to the threshold before I can say this is the right fix, just in case the way to solve PR31613 applies here as well.
I'm also worried that if reaching any such threshold is legitimate, an assertion is not the right way to address this in the first place.


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

https://reviews.llvm.org/D103016



More information about the llvm-commits mailing list