[PATCH] [X86] replace vinsertf128 intrinsics with generic shuffles

Sanjay Patel spatel at rotateright.com
Thu Mar 5 17:25:19 PST 2015


================
Comment at: lib/IR/AutoUpgrade.cpp:639
@@ +638,3 @@
+      Value *Op1 = CI->getArgOperand(1);
+      unsigned Imm = cast<ConstantInt>(CI->getArgOperand(2))->getZExtValue();
+      VectorType *VecTy = cast<VectorType>(CI->getType());
----------------
andreadb wrote:
> What happens if Imm is for example 2 or 4?
> 
> According to the Intel documentation: "The high 7 bits of the immediate are ignored". So, only the first bit of 'Imm' has a meaning in this context.
> However, your code doesn't clear the upper bits of 'Imm'. So (unless I misread the code) the for loops between lines 665 and 674 would propagate the wrong indices if Imm is an even number bigger than zero.
> 
> Can you add a test for the case where Imm is a non-zero even number?
Yes, that would be a bug. We need to ignore the high bits.

The software intrinsic doesn't match the hardware either; it shows an 'int' rather than a 'char'.

http://reviews.llvm.org/D8086

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list