A value can legitimately appear more than 100 times. This assert in
general seems bogus to me, but raise it to 100 times the number of
blocks in the function which is still arbitrary.
Addresses bug 50444.
Paths
| Differential D103016
NewGVN: Relax assertion about number of times a value is seen Needs ReviewPublic Authored by arsenm on May 24 2021, 6:23 AM.
Details Summary A value can legitimately appear more than 100 times. This assert in Addresses bug 50444.
Diff Detail
Unit TestsFailed
Event TimelineComment Actions Add @asbirlea since she is also working on another similar issue PR31613. sounds like the test case hit some inefficiency of the way we are processing the instructions. The %i678 = and other similar geps against %i were processed again and again. Comment Actions 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. Comment Actions I cannot reproduce the bug. I had to decrease the counter down to 6 in order to activate the assertion. This is normal. The counter is updated after value numbering. An instruction will be processed again if one of its operands is added in a different congruence class. Do you remember which was the problem? Comment Actions
I was able to reproduce this with the testcase attached to the issue a few weeks ago, I don’t remeymore than that
Revision Contents
Diff 347371 llvm/lib/Transforms/Scalar/NewGVN.cpp
llvm/test/Transforms/NewGVN/bug50444.ll
|