[PATCH] D51236: [InstCombine] Extend (add (sext x), cst) --> (sext (add x, cst')) and (add (zext x), cst) --> (zext (add x, cst')) to work for vectors
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 27 15:13:04 PDT 2018
spatel added a comment.
I appreciate the concern about having a test for constant expressions if we just use m_Constant() here, but I wouldn't hold this patch up for that. We have dozens of uses of m_Constant() and barely any tests for constant expressions (presumably because those are rarely the motivation for adding a transform).
The rarity of constant expressions would also suggest that we don't really need to be worried about efficiency in handling those.
So my preference would be:
1. Add the tests with baseline checks.
2. Switch this transform over to m_Constant(), so it can handle arbitrary vector constants.
3. Add m_ConstantIntOrConstantIntVec and apply it wherever we think it would make the code clearer/faster. I'm not convinced yet that it's worth the effort to exclude constant expressions, but if it is, then let's do it throughout instcombine?
Repository:
rL LLVM
https://reviews.llvm.org/D51236
More information about the llvm-commits
mailing list