[PATCH] D70844: [InstCombine] Fix big-endian miscompile of (bitcast (zext/trunc (bitcast)))

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 29 10:06:08 PST 2019


lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:1851-1894
   // Now that the element types match, get the shuffle mask and RHS of the
   // shuffle to use, which depends on whether we're increasing or decreasing the
   // size of the input.
   SmallVector<uint32_t, 16> ShuffleMask;
   Value *V2;
 
+  bool IsBigEndian = IC.getDataLayout().isBigEndian();
----------------
I personally find this to be a somewhat too convoluted approach.
What do you think about: (didn't compile-test)
```
  SmallVector<uint32_t, 16> ShuffleMaskStorage;
  ArrayRef<uint32_t> ShuffleMask;
  Value *V2;

  bool IsBigEndian = IC.getDataLayout().isBigEndian();
  unsigned SrcElts = SrcTy->getNumElements();
  unsigned DestElts = DestTy->getNumElements();

  // Produce identity shuffle mask for src vector
  ShuffleMaskStorage.resize(SrcElts);
  std::iota(ShuffleMaskStorage.begin(), ShuffleMaskStorage.end(), 0);

  if (SrcElts > DestElts) {
    // If we're shrinking the number of elements, just shuffle in the elements
    // from the input and use undef as the second shuffle input.
    V2 = UndefValue::get(SrcTy);
    ShuffleMask = ShuffleMaskStorage;
    unsigned EltDelta = SrcElts - DestElts;
    // Vector in-memory layout differs between little- and big- endian machines.
    if(IsBigEndian)
      ShuffleMask.drop_front(EltDelta)
    else
      ShuffleMask.drop_back(EltDelta)
  } else {
    // If we're increasing the number of elements, shuffle in all of the
    // elements from InVal. Fill the rest of the result elements with zeros
    // from a constant zero.
    V2 = Constant::getNullValue(SrcTy);

    // Vector in-memory layout differs between little- and big- endian machines.
    decltype(ShuffleMaskStorage)::iterator InsertPos = IsBigEndian ?
                                                       ShuffleMaskStorage.begin() :
                                                       ShuffleMaskStorage.end()
    
    // And pad with zeros
    unsigned EltDelta = DestElts - SrcElts;
    ShuffleMaskStorage.insert(InsertPos, EltDelta, /*first element of second shuffle input*/SrcElts)
    
    ShuffleMask = ShuffleMaskStorage;
  }

  return new ShuffleVectorInst(InVal, V2,
                               ConstantDataVector::get(V2->getContext(),
                                                       ShuffleMask));
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70844





More information about the llvm-commits mailing list