Fix vector splitting for extract_vector_elt and vector elements of <8-bits.

[dsanders]

Summary

One of the vector splitting paths for extract_vector_elt tries to lower:

define i1 @via_stack_bug(i8 signext %idx) {
  %1 = extractelement <2 x i1> <i1 false, i1 true>, i8 %idx
  ret i1 %1
}

to:

define i1 @via_stack_bug(i8 signext %idx) {
  %base = alloca <2 x i1>
  store <2 x i1> <i1 false, i1 true>, <2 x i1>* %base
  %2 = getelementptr <2 x i1>, <2 x i1>* %base, i32 %idx
  %3 = load i1, i1* %2
  ret i1 %3
}

However, the elements of <2 x i1> are not byte-addressible. The result of this
is that the getelementptr expands to '%base + %idx * (1 / 8)' which simplifies
to '%base + %idx * 0', and then simply '%base' causing all values of %idx to
extract element zero.

This commit fixes this by promoting the vector elements of <8-bits to i8 before
splitting the vector.

This fixes a number of test failures in pocl.

[pekka.jaaskelainen]

The fix looks good, but maybe the test should be an .ir test as this probably affects other targets than MIPS too? Also could be a candidate for 3.7.1?

[dsanders]

The fix looks good, but maybe the test should be an .ir test as this probably affects other targets than MIPS too?

It can affect other targets but we can't make a generic test because the legalization settings for the target determine whether the bug is encountered. If v2i1 is legal or the legalizer widens/promotes the vector instead of splitting it, the bug won't occur.

Also could be a candidate for 3.7.1?

Yes, I'm planning to nominate it for 3.7.1 once it's committed.

lib/CodeGen/SelectionDAG/LegalizeTypes.cpp

 SDValue DAGTypeLegalizer::GetVectorElementPointer(SDValue VecPtr, EVT EltVT,
                                                   SDValue Index) {
   SDLoc dl(Index);
   // Make sure the index type is big enough to compute in.
   Index = DAG.getZExtOrTrunc(Index, dl, TLI.getPointerTy(DAG.getDataLayout()));
 
   // Calculate the element offset and add it to the pointer.
   unsigned EltSize = EltVT.getSizeInBits() / 8; // FIXME: should be ABI size.
+  assert(EltSize * 8 == EltVT.getSizeInBits() &&
+         "Converting bits to bytes lost precision");
 
   Index = DAG.getNode(ISD::MUL, dl, Index.getValueType(), Index,
                       DAG.getConstant(EltSize, dl, Index.getValueType()));
   return DAG.getNode(ISD::ADD, dl, Index.getValueType(), Index, VecPtr);
 }
 
 /// JoinIntegers - Build an integer with low bits Lo and high bits Hi.
 SDValue DAGTypeLegalizer::JoinIntegers(SDValue Lo, SDValue Hi) {

lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

return SDValue(DAG.UpdateNodeOperands(N, Hi,
                                   DAG.getConstant(IdxVal - LoElts, SDLoc(N),
                                                   Idx.getValueType())), 0);
   }
 
   // See if the target wants to custom expand this node.
   if (CustomLowerNode(N, N->getValueType(0), true))
     return SDValue();
 
-  // Store the vector to the stack.
-  EVT EltVT = VecVT.getVectorElementType();
+  // Make the vector elements byte-addressable if they aren't already.
   SDLoc dl(N);
+  EVT EltVT = VecVT.getVectorElementType();
+  if (EltVT.getSizeInBits() < 8) {
+    SmallVector<SDValue, 4> ElementOps;
+    for (unsigned i = 0; i < VecVT.getVectorNumElements(); ++i) {
+      ElementOps.push_back(DAG.getAnyExtOrTrunc(
+          DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl, EltVT, Vec,
+                      DAG.getConstant(i, dl, MVT::i8)),
+          dl, MVT::i8));
+    }
+
+    EltVT = MVT::i8;
+    VecVT = EVT::getVectorVT(*DAG.getContext(), EltVT,
+                             VecVT.getVectorNumElements());
+    Vec = DAG.getNode(ISD::BUILD_VECTOR, dl, VecVT, ElementOps);
+  }
+
+  // Store the vector to the stack.
   SDValue StackPtr = DAG.CreateStackTemporary(VecVT);
   SDValue Store = DAG.getStore(DAG.getEntryNode(), dl, Vec, StackPtr,
                                MachinePointerInfo(), false, false, 0);
 
   // Load back the required element.
   StackPtr = GetVectorElementPointer(StackPtr, EltVT, Idx);
   return DAG.getExtLoad(ISD::EXTLOAD, dl, N->getValueType(0), Store, StackPtr,
                         MachinePointerInfo(), EltVT, false, false, false, 0);

test/CodeGen/Mips/llvm-ir/extractelement.ll

+; RUN: llc < %s -march=mips -mcpu=mips2 | FileCheck %s -check-prefix=ALL
+
+; This test triggered a bug in the vector splitting where the type legalizer
+; attempted to extract the element with by storing the vector, then reading
+; an element back. However, the address calculation was:
+;   Base + Index * (EltSizeInBits / 8)
+; and EltSizeInBits was 1. This caused the index to be forgotten.
+define i1 @via_stack_bug(i8 signext %idx) {
+  %1 = extractelement <2 x i1> <i1 false, i1 true>, i8 %idx
+  ret i1 %1
+}
+
+; ALL-LABEL: via_stack_bug:
+; ALL-DAG:       addiu  [[ONE:\$[0-9]+]], $zero, 1
+; ALL-DAG:       sb     [[ONE]], 7($sp)
+; ALL-DAG:       sb     $zero, 6($sp)
+; ALL-DAG:       addiu  [[VPTR:\$[0-9]+]], $sp, 6
+; ALL-DAG:       addu   [[EPTR:\$[0-9]+]], $4, [[VPTR]]
+; ALL:           lbu    $2, 0([[EPTR]])