[PATCH] D91589: [DAGCombiner] Fold (sext (not i1 x)) -> (add (zext i1 x), -1)

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 4 11:28:36 PST 2020


spatel added a comment.

In D91589#2431862 <https://reviews.llvm.org/D91589#2431862>, @laytonio wrote:

>> are we excluding vector types intentionally (do we have any test coverage?) or is that planned as a follow-up? Either way, we should add a code comment to explain.
>
> The thought behind excluding vector types was that materializing the all ones constant may be more expensive in some cases. Although thinking about it again, I'm not sure if that is really true as we would remove the constant we are xoring against, most likely making it a wash. I did try allowing vector types and it caused a regression in a single test. That regression did seem like it was avoidable though as it exposed a missing fold of `(not (xor (setcc), (setcc))) -> (xor (not (setcc)), (setcc))`. Implementing that turned into a bit of a rabbit hole though. What do you think the best approach is here?

I think it's ok to proceed with the scalar-only patch to make progress, but put a TODO comment on it about extending to vectors.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91589/new/

https://reviews.llvm.org/D91589



More information about the llvm-commits mailing list