CHI should only be inserted to a basic block properly dominating its successor.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/Transforms/Scalar/GVNHoist.cpp | ||
---|---|---|
802 ↗ | (On Diff #122030) | I haven't reviewed in detail, but I'm skeptical this check actually does the right thing in general. A "Loop*" is specifically a loop with a single entry point, so this check won't handle irreducible control flow correctly. |
lib/Transforms/Scalar/GVNHoist.cpp | ||
---|---|---|
802 ↗ | (On Diff #122030) | Looking a little more, it might not be relevant... I'm not following the algorithm for insertion points, and I don't really have time to dig deeper, so please ignore me. :) |
lib/Transforms/Scalar/GVNHoist.cpp | ||
---|---|---|
802 ↗ | (On Diff #122030) | Thanks! |
lib/Transforms/Scalar/GVNHoist.cpp | ||
---|---|---|
801 ↗ | (On Diff #122030) | I don't think this check is overall safe. The definition of loop here is not going to be sufficient either. I think if you want to avoid spurious CHI's, you need to do the equivalent of what you see in "A Practical Improvement to the Partial Redundancy Elimination in SSA Form" (https://sites.google.com/site/jongsoopark/home/ssapre.pdf) which precisely defines which will be unnecessary and the safety conditions. |
lib/Transforms/Scalar/GVNHoist.cpp | ||
---|---|---|
801 ↗ | (On Diff #122030) | Thanks for the reference, I'll study the paper and find out if they have some extra safety checks. |