[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