Separate the check for blend shuffle_vector masks into isBlendMask.
This function will also be used to check if a vector shuffle is legal. No
change in functionality was intended, but we ended up improving codegen on
two tests, which were being (more) optimized only if the resulting shuffle
was legal.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Hi Filipe,
By adding the call 'isBlendMask' to 'X86TargetLowering::isShuffleMaskLegal' you actually made a functional change.
With you change, the DAGCombiner now is able to fold more vector OR dag nodes according to rules:
fold (or (shuf A, V_0, MA), (shuf B, V_0, MB)) -> (shuf A, B, Mask1) fold (or (shuf A, V_0, MA), (shuf B, V_0, MB)) -> (shuf A, B, Mask1).
I think that the behavior change in the DAGCombiner is good (as you said, we optimize more cases; that explains why you had to "fix" test 'combine-or.ll').
I can see other uses of method 'isShuffleMaskLegal' in LegalizeDAG.cpp and LegalizeVectorOps.cpp.
For example, in LegalizeDAG.cpp, method SelectionDAGLegalize::ExpandBUILD_VECTOR calls 'isShuffleMaskLegal' to see if it is possible to expand build_vector dag nodes into legal shuffles.
The other place where we call 'isShuffleMaskLegal' is LegalizeVectorOps.cpp.
More specifically, method ExpandBSWAP uses it to check if we can generate a legal byte wise shuffle to implement a BSWAP.
That said, I cannot see how your change might have negatively affected those methods.
For what is worth, your change looks good to me (it definitely makes sense for the DAGCombiner).