[llvm-commits] [llvm] r139479 - in /llvm/trunk: include/llvm/Target/TargetSelectionDAG.td lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86ISelLowering.h lib/Target/X86/X86InstrSSE.td

[Bruno Cardoso Lopes]

2011/9/11 Rotem, Nadav <nadav.rotem at intel.com>:
> Hi Bruno,
>
> Thanks for the review.
>
>>> +def : Pat<(v16i8 (X86blendv (v16i8 VR128:$src1), (v16i8 VR128:$src2),
>>> + VR128:$mask)), (VPBLENDVBrr VR128:$src1, VR128:$src2, VR128:$mask)>;
>>Why is this pattern above still here? Haven't you removed the BLENDV
>>node? Please remove this here and it from X86InstrFragments.td
>
> Okay. I will remove them.  BTW, do we need to do anything for FastISel ?

Not that I'm aware!

>>> +  def : Pat<(v2f64 (vselect (v2i64 XMM0), (v2f64 VR128:$src1),
>>> + (v2f64 VR128:$src2))), (BLENDVPDrr0 VR128:$src1, VR128:$src2)>;
>>>  }
>
>>Align the declaration of the patterns in the same it was before to be
>>consistent with the other ones in the .td file. Also please
>>canonicalize the mask to be last operand, as it was before.
>
> I will change the alignment. I don't understand the second comment. Do you want me to change the order of the operands in ISD::VSELECT ?

Cool. I was confused because of the BLENDV order, nevermind.
Thanks Nadav,

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc