[PATCH] D102393: [DAGCombiner] Relax an assertion to an early return

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added a comment.

In D102393#2757020 <https://reviews.llvm.org/D102393#2757020>, @RKSimon wrote:

> Have you investigated whether we can improve support for implicit truncation of build vector element on this path instead of just bailing out?

Yeah I took a look. This transform later implicitly forbids implicit truncation via its use of `isAllOnesOrAllOnesSplat` and `isNullOrNullSplat` so that would need changed. I don't see why it couldn't support implicit truncation as long as it doesn't blindly take one of the constant vector elements assuming it's the same type as the resulting vector element type. The question is also whether we can get a test case, given this combine also runs before legalization where we (RISC-V) wouldn't yet have the implicit truncation.

That said, it seems like this restriction on implicit truncation is a common idiom in the DAGCombiner -- given by the number of uses of `isConstantOrConstantVector` alone -- so maybe that's a larger discussion? There's so much almost-duplication in SelectionDAG for "isBuildVector", "isSplat", "isBuildOrSplat", "isConstantSplat", "isConstantSplatAllOnes", etc., that I don't think anyone wants to start untangling that web.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102393