[PATCH] D81661: [VectorCombine] scalarize compares with insertelement operand(s)

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked an inline comment as done.
spatel added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:324-325
 
+  // Do not convert the vector condition of a vector select into a scalar
+  // condition. That is likely to cause problems for codegen.
+  bool IsCmp = Pred != CmpInst::Predicate::BAD_ICMP_PREDICATE;
----------------
lebedev.ri wrote:
> This needs more words i'd say.
> 
> https://godbolt.org/z/4tqgwP
> I believe the problem we are preventing is the differences in bit patterns for true/false between scalar and vector comparisons.
> * https://www.felixcloutier.com/x86/setcc `Sets the destination operand to 0 or 1 depending on the settings of the status flags`
> * https://www.felixcloutier.com/x86/pcmpeqb:pcmpeqw:pcmpeqd `If a pair of data elements is equal, the corresponding data element in the destination operand is set to all 1s`
> * https://www.felixcloutier.com/x86/blendvps `depending on mask bits defined in the mask register operand. The mask bits are the most significant bit in each dword element of the mask register.`
> 
> And there are other variants on other targets. (there's enum, can't find it right now)
> 
> Can we just model that new extra cost?
Yes, the bool format (enum BooleanContent in SDAG) is one problem. The other is the potential transfer between register files as the x86 example demonstrates.

IMO, this is similar to the existing bailout for loaded operands below here. We can try to improve the cost model to deal with larger patterns, but the ability to handle this situation doesn't exist currently AFAIK. We don't have a way to specify the combined cost of the not-yet-created scalar op + insertelt with existing users of the vector op.


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

https://reviews.llvm.org/D81661