[PATCH] Generate better code for shuffles

Sanjay Patel spatel at rotateright.com
Tue Dec 16 10:42:22 PST 2014


In http://reviews.llvm.org/D6678#101948, @mkuper wrote:

> By the way, Sanjay, I've verified that the test-cases you have in vec_extract-avx.ll pass with this.


Excellent! I'll leave it to the other reviewers to decide if we need a TLI hook for this; otherwise, LGTM.

This patch is better than http://reviews.llvm.org/D6622 because it handles cases that that one bails out on. Eg, when we're extracting across the 128-bit boundary as in your test case or:

  define void @low_v8f32_to_v4f32(<8 x float> %v, <4 x float>* %ptr) {
   %ext0 = extractelement <8 x float> %v, i32 1   
   %ext1 = extractelement <8 x float> %v, i32 2
   %ext2 = extractelement <8 x float> %v, i32 3
   %ext3 = extractelement <8 x float> %v, i32 4   ; crossing the 128-bit lane
   %ins0 = insertelement <4 x float> undef, float %ext0, i32 0
   %ins1 = insertelement <4 x float> %ins0, float %ext1, i32 1
   %ins2 = insertelement <4 x float> %ins1, float %ext2, i32 2
   %ins3 = insertelement <4 x float> %ins2, float %ext3, i32 3
   store <4 x float> %ins3, <4 x float>* %ptr, align 16
   ret void
  }

You're avoiding the x86 miscompile that I saw by only generating extract_subvectors along half-vector-length boundaries. The case that I saw failing would be caused by a node like this:

  DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, VT, VecIn1, DAG.getIntPtrConstant(1)); // not a half-vector extract

When that ends up in X86ISelLowering's Extract128BitVector(), we quietly round the index down with this integer math:

  unsigned NormalizedIdxVal = (((IdxVal * ElVT.getSizeInBits()) / vectorWidth) * ElemsPerChunk);

The comment around there says:

  // Idx is an index in the 128 bits we want. It need not be aligned to a 128-bit boundary.

While that matches what the integer math does, I think this code should be asserting that the index is a correct multiple of the number of elements. Otherwise, we're generating an extract that doesn't execute what the input node is asking for.


http://reviews.llvm.org/D6678

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






More information about the llvm-commits mailing list