[PATCH] D13505: [X86] Fix bad treatment of multi-lane blends in BUILD_VECTORtoBlendMask()

[Andrea Di Biagio via llvm-commits]

andreadb accepted this revision.
andreadb added a reviewer: andreadb.
andreadb added a comment.
This revision is now accepted and ready to land.

Hi Michael

The patch looks good to me.

Unrelated to your patch (but still important in my opinion).

Currently, function BUILD_VECTORtoBlendMask is only called by function 'transformVSELECTtoBlendVECTOR_SHUFFLE'. The goal of that function is (if possible) to convert a VSELECT into a target independent vector shuffle node.

Function transformVSELECTtoBlendVECTOR_SHUFFLE performs the folling steps:

1. At first it checks if the VSELECT condition is constant (i.e. a build_vector of ConstantSDNode (or Undef));
2. It then delegates to function 'BUILD_VECTORtoBlendMask' the computation of a bitmask (an unsigned value) which represents some sort of 'compressed' vector shuffle mask;
3. The bitmask is eventually decoded into a mask which is suitable for a target independent vector shuffle node.

The more I look at that code, the more I think that point 2. could have been entirely avoided.
What I am trying to say (correct me if I am wrong) is that we probably don't need to go through a two-step process where we firstly convert the select mask to a bitmask, and then we convert the bitmask to a vector shuffle mask...

Presumably this design made sense in the past (more than one year ago) since we didn't have the new shuffle lowering logic in place. Nowadays we should probably check if it would make more sense to just remove function BUILD_VECTORtoBlendMask and simplify the logic in transformVSELECTtoBlendVECTOR_SHUFFLE. This is just an idea and - as I said - unrelated to your patch.. I hope it makes sense.

-Andrea


http://reviews.llvm.org/D13505