ExpandBinOp folds (A op' B) op C by transforming it into
(A op C) op' (B op C) first, simplifying the two subexpressions
(say the result as L and R), and folding L op' R if possible.
However, if C contains undef, this results in making undef have two different
values whereas the original expression may not;
as a simplest case, transforming undef * (1 + 1) into undef + undef isn't
safe, because the former one is an even whereas the latter one is any number.
This patch makes ExpandBinOp fold only when it's safe.
- If either L or R is special in that L op' ? or ? op' R always folds into a constant, it is safe to fold the expression to the constant.
- If C is never undef or poison, it is safe.
At https://reviews.llvm.org/D83360#2146539 , I proposed making InstSimplify
remember to which value undefs are folded & use the cached value if the undef
is met again.
My plan was to implement this solution by adding a static ValueOrUse class
(so we can distinguish undefs by Use) and letting InstSimplify's functions
use it instead of Value. But, it had several problems.
- If InstSimplify fails to simplify values, the record of folded undefs should be rewinded at function returns. Doing this all over InstSimplify is error-prone and easy to miss.
- Sometimes the value that undef is folded isn't just a constant. For example, if undef + 1 <= X is folded into true, the undef is folded into either -1 or a random number between [0, X-1]. Recording this isn't trivial.
- Constant folding can fold undefs too. This causes changes in the relevant libraries.
- Finally, if InstSimplify becomes smart enough, this still can cause miscompilation. For example, if InstSimplify skims through instructions in the same basic block and find equivalent expressions, a miscompilation can happen without explicit folding of undefs.
Another approach that I tried was to simply
stopping undef folds when called from ExpandBinOp recursively.
However, this also required a nontrivial change because two
UndefValue instances compare equal, leading to undesirable foldings like
undef - undef --> 0.
Also, it still does not resolve the possible miscompilation problem.