[llvm] [InstCombine] Tighten use constraint in factorization transforms (PR #102943)

Kevin McAfee via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 15 10:56:28 PDT 2024


kalxr wrote:

> > The transform in `tryFactorization()` used to require that both operations have one use and thus would be removable after the transform. Commit [0cfc651](https://github.com/llvm/llvm-project/commit/0cfc6510323fbb5a56a5de23cbc65f7cc30fd34c) loosened factorization constraints such that only one of the operations must be removable after the transform. There are cases where this will increase the number of non-intermediate variable (variable that is not the result of a binop involved in the transform) uses with no discernable benefit (e.g. https://alive2.llvm.org/ce/z/8K4VuE). Likewise, `factorizeMathWithShlOps()` uses the same constraints and has the same issue (https://alive2.llvm.org/ce/z/EKK-Kp).
> 
> Could you explain in more detail why these transforms are undesirable (to the point that we must actively prevent them)?
> 
> In https://alive2.llvm.org/ce/z/8K4VuE `%add` after the transform only depends on one multiply, instead of two, which seems like a good change in principle?

I've added another precommit test that should help demonstrate an issue of the transform (mul_add_chain_common_factor_uses). This is a case where applying the transform eliminates no instructions while removing redundancy that can be eliminated in the future by e.g. GVN. https://alive2.llvm.org/ce/z/zeNCwD shows this - if you apply InstCombine then GVN, only 2 instructions are removed. If you remove InstCombine and only run GVN, 4 instructions are removed.

https://github.com/llvm/llvm-project/pull/102943


More information about the llvm-commits mailing list