[PATCH] Separate the check for blend shuffle_vector masks

Andrea Di Biagio Andrea_DiBiagio at sn.scee.net
Fri May 30 10:11:25 PDT 2014


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).

http://reviews.llvm.org/D3964






More information about the llvm-commits mailing list