[DAGCombiner] [AMDGPU] Allow vector loads in MatchLoadCombine

[jrbyrnes]

Summary

Since SROA chooses promotion based on reaching load / stores of allocas, we may run into scenarios in which we alloca a vector, but promote it to an integer. The result of which is the familiar LoadCombine pattern (i.e. ZEXT, SHL, OR). However, instead of coming directly from distinct loads, the elements to be combined are coming from ExtractVectorElements which stem from a shared load.

This patch identifies such a pattern and combines it into a load.

Change-Id: I0bc06588f11e88a0a975cde1fd71e9143e6c42dd

[arsenm]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

7970

Why implicitly cast to int to assert the value? Just use uint64_t?

7978

Early return on condition instead of wrapping ternary operator

llvm/test/CodeGen/AMDGPU/combine-vload-extract.ll

2

Don’t need -O3?

[jrbyrnes]

Address comments.

[jrbyrnes]

Adding reviewers for increased perspective.

[jmmartinez]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

8000

Is is normal that the conditions is checking if VectorIndex is 0 ? Shouldn't it be auto BPVectorIndex = VectorIndex.value_or(0U); ?

8314

This variable seems unused.

[RKSimon]

precommit combine-vload-extract.ll with current (trunk) codegen and rebase the patch to show the codegen diff

[spatel]

Both SLP and VectorCombine should try to make patterns like this better in IR, so there might be some target cost/legality checks that need adjusting.
There's also an in-progress patch for -aggressive-instcombine that could be relevant:
D127392

Would it be better to transform this before codegen?
https://alive2.llvm.org/ce/z/uyxHSW

[jrbyrnes]

In D133584#3799669, @spatel wrote:

Both SLP and VectorCombine should try to make patterns like this better in IR, so there might be some target cost/legality checks that need adjusting.
There's also an in-progress patch for -aggressive-instcombine that could be relevant:
D127392

Would it be better to transform this before codegen?
https://alive2.llvm.org/ce/z/uyxHSW

Hi, thanks for your comment! The reason I tagged you is because you seem to be involved in the most closely related issues to the one here (D67841, https://bugs.llvm.org/show_bug.cgi?id=42708). It seems the conclusion is to have vectorization passes (and optimization passes in general) leave LoadCombine patterns untouched, and resolve it in the backend, no? That was the logic I used for the design here.

On the other hand, it seems D127392 is using the opposite design approach. Is the current consensus to do load combining in optimizer?

At a glance, D127392 will not address the issue identified here because it does not handle vector loads.

[spatel]

In D133584#3800232, @jrbyrnes wrote:

In D133584#3799669, @spatel wrote:

Both SLP and VectorCombine should try to make patterns like this better in IR, so there might be some target cost/legality checks that need adjusting.
There's also an in-progress patch for -aggressive-instcombine that could be relevant:
D127392

Would it be better to transform this before codegen?
https://alive2.llvm.org/ce/z/uyxHSW

Hi, thanks for your comment! The reason I tagged you is because you seem to be involved in the most closely related issues to the one here (D67841, https://bugs.llvm.org/show_bug.cgi?id=42708). It seems the conclusion is to have vectorization passes (and optimization passes in general) leave LoadCombine patterns untouched, and resolve it in the backend, no? That was the logic I used for the design here.

On the other hand, it seems D127392 is using the opposite design approach. Is the current approach to do load combining in optimizer?

LLVM has gone back and forth on this. There was a general load combine pass for IR, but it was removed because it interfered with other transforms in IR. So we started hacking away at codegen instead, but there are programs where doing the transform in codegen is too late to get the optimal results. So we have some limited transforms in the vectorization passes, and now we're trying to reintroduce load combining as a canonicalization (but in very limited cases and gated by target-specific legality checks).

At a glance, D127392 will not address the issue identified here because it does not handle vector loads.

Right - getting that to work correctly on the most basic integer load patterns is the first step, but we could enhance the transform for more cases (hopefully without too much work).

[jrbyrnes]

LLVM has gone back and forth on this. There was a general load combine pass for IR, but it was removed because it interfered with other transforms in IR. So we started hacking away at codegen instead, but there are programs where doing the transform in codegen is too late to get the optimal results. So we have some limited transforms in the vectorization passes, and now we're trying to reintroduce load combining as a canonicalization (but in very limited cases and gated by target-specific legality checks).

With this in mind, perhaps the most consistent / best way to handle this pattern is to catch it in CodeGen (this patch), and, in a separate patch, handle this pattern in a vectorization / instcombine pass (gated by legality checks). It seems to that catching it in CodeGen will only help things (e.g. in scenarios where it is not handled by transform passes).

[spatel]

In D133584#3800314, @jrbyrnes wrote:

LLVM has gone back and forth on this. There was a general load combine pass for IR, but it was removed because it interfered with other transforms in IR. So we started hacking away at codegen instead, but there are programs where doing the transform in codegen is too late to get the optimal results. So we have some limited transforms in the vectorization passes, and now we're trying to reintroduce load combining as a canonicalization (but in very limited cases and gated by target-specific legality checks).

With this in mind, perhaps the most consistent / best way to handle this pattern is to catch it in CodeGen (this patch), and, in a separate patch, handle this pattern in a vectorization / instcombine pass (gated by legality checks). It seems to that catching it in CodeGen will only help things (e.g. in scenarios where it is not handled by transform passes).

Sure - I didn't look at the diffs closely, but I don't object to improving the SDAG implementation. Just wanted to let you know that there are potential other places to try this kind of transform.

[jrbyrnes]

Address review comments -- update usage of Optional API.

[jrbyrnes]

Sure - I didn't look at the diffs closely, but I don't object to improving the SDAG implementation. Just wanted to let you know that there are potential other places to try this kind of transform.

Thanks, I appreciate your feedback & thoughts on this -- especially with regard to the approach -- as you seem to be very knowledgeable about the design decisions for this issue.

Anyway, it sounds like the approach for now is to land this upstream. Are there additional thoughts about the implementation details, or is this ready?

[spatel]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

7885–7889

I added more tests with:
ef7d61d67cb9
...so please update the auto-generated checks there.

It might help to add some test or code comments to explain the transforms in those examples. Just looking at this code, I can't tell what the relationship is between the 3 index parameters.

7899

and -> an?

8224

Can this just be updated to use getScalarSizeInBits()?

[jrbyrnes]

Address review comments.

Update tests in ef7d61d67cb9

[jrbyrnes]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

7885–7889

Thanks for taking a look! I updated the tests you added, please let me know if you need additional info on the results.

[spatel]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

7894

Check that Depth != 0 instead of adding the Root parameter?

8293–8294

Still need to resolve this - if this can't be:

unsigned LoadBitWidth = P.Load->getMemoryVT().getScalarSizeInBits();

...then please add a test to demonstrate how that fails or a code comment to explain why the simpler code is not valid.

8342–8343

Do we have a test where VectorOffset is non-zero? If not, please add one (add baseline tests as needed, no pre-commit review is necessary).

[jrbyrnes]

Address review comments (remove unnecessary "Root" parameter).

[spatel]

I'm still not confident in my understanding of the various index values even with the added code comments.
I'll try to step through some of these tests in the debugger to get a better idea, but it would be good if another reviewer can have a look too for a second opinion.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

8342–8343

This is marked as done, but I can't tell from just looking at the tests which one exercises that path. Add an example/comment here or next to a test, so it's clearer what this looks like?

[RKSimon]

llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h

52

Would it be better to replace this with addToOffset() that adds to the existing offset (or set it if == None)?

[jrbyrnes]

replace setOffset with addToOffset

[jrbyrnes]

In D133584#3818235, @spatel wrote:

I'm still not confident in my understanding of the various index values even with the added code comments.
I'll try to step through some of these tests in the debugger to get a better idea, but it would be good if another reviewer can have a look too for a second opinion.

Hi -- thanks for your comments and reviews. I realize I never submitted my last round of comments, so I've done so here. Apologies for the delay -- this has probably added to the incomplete understanding.

I have included a description of the various index parameters via example in an inline comment.

llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h

52

That does make more sense, thanks!

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

7894

Thanks for pointing this out!

In fact, this patch did not add the Root parameter, but it does look unnecessary, so I think it makes sense to remove it along with the other changes in this patch.

8342–8343

The non-zero VectorOffset path is covered by any test that uses a ExtractElement with a non-zero index. As an example, extractelement <4 x i8> %ld, i32 1 will have a VectorOffset of 1.

CalculateByteProvider will only allow such a VectorOffset if we are trying to Provide for the 1th byte. We enforce this by making sure the StartingIndex == VectorOffset. The idea is to match {0, 1, 2, 3} bytes with VectorLoad -> ExtractElement {0, 1, 2, 3}. If we find an ExtractElement index that does not match the VectorOffset, we conservatively assume that we are shuffling the elements in the vector and can not combine into a load.

So this covers the VectorIndex (e.g. VectorOffset) and StartingIndex parameters in CalculateByteProvider. The remaining parameter is the original Index parameter. This parameter ensures the shift and loadwidth we find are able to provide for the relevant byte. For example, if we are trying to provide for the 2th byte, then we must find either a Load 8+bit -> SHL 16, or Load 16+bit -> SHL 8, or Load 24+bit. Basically, the combination of shift and byte width must cover the byte we are trying to provide for. If so, then the check (Index >= NarrowByteWidth) will be false, and we will return the ByteProvider.

[RKSimon]

llvm/test/CodeGen/AMDGPU/combine-vload-extract.ll

75

Please can you pre-commit these to trunk and then rebase to show the codegen change from this patch/

[jrbyrnes]

Rebase

[jrbyrnes]

Extend ByteProvider / VectorOffset handling to support vectorScalarTypes > 1 Byte.

Additional comments, tests

[jrbyrnes]

Rebase on top of precommitted tests.

[spatel]

I still can't say that I see all of the potential corner cases, but the extra comments help to explain what's going on, and it seems to work as expected, so LGTM.
Might still be good to get a 2nd approval from another reviewer since several people have commented.

[jrbyrnes]

In D133584#3830968, @spatel wrote:

I still can't say that I see all of the potential corner cases, but the extra comments help to explain what's going on, and it seems to work as expected, so LGTM.
Might still be good to get a 2nd approval from another reviewer since several people have commented.

Hey @spatel , thanks! I appreciate your help and thoughts on this patch. I'll keep the review up for a bit longer while I do some testing to see if anyone else is willing to approve.

[arsenm]

LGTM with nit

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

7895

deMorgan this

[jrbyrnes]

Resolve nit, fix a few comment typos. NFC

[jrbyrnes]

Landed via rGcebec4208982

llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h

BaseIndexOffset(SDValue Base, SDValue Index, int64_t Offset,
                   bool IsIndexSignExt)
       : Base(Base), Index(Index), Offset(Offset),
         IsIndexSignExt(IsIndexSignExt) {}
 
   SDValue getBase() { return Base; }
   SDValue getBase() const { return Base; }
   SDValue getIndex() { return Index; }
   SDValue getIndex() const { return Index; }
+  void addToOffset(int64_t VectorOff) {
+    Offset = Offset.value_or(0) + VectorOff;
+  }
   bool hasValidOffset() const { return Offset.has_value(); }
   int64_t getOffset() const { return *Offset; }
 
   // Returns true if `Other` and `*this` are both some offset from the same base
   // pointer. In that case, `Off` is set to the offset between `*this` and
   // `Other` (negative if `Other` is before `*this`).
   bool equalBaseIndex(const BaseIndexOffset &Other, const SelectionDAG &DAG,
                       int64_t &Off) const;

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

 /// Represents known origin of an individual byte in load combine pattern. The
 /// value of the byte is either constant zero or comes from memory.
 struct ByteProvider {
   // For constant zero providers Load is set to nullptr. For memory providers
   // Load represents the node which loads the byte from memory.
   // ByteOffset is the offset of the byte in the value produced by the load.
   LoadSDNode *Load = nullptr;
   unsigned ByteOffset = 0;
+  unsigned VectorOffset = 0;
 
   ByteProvider() = default;
 
-  static ByteProvider getMemory(LoadSDNode *Load, unsigned ByteOffset) {
-    return ByteProvider(Load, ByteOffset);
+  static ByteProvider getMemory(LoadSDNode *Load, unsigned ByteOffset,
+                                unsigned VectorOffset) {
+    return ByteProvider(Load, ByteOffset, VectorOffset);
   }
 
-  static ByteProvider getConstantZero() { return ByteProvider(nullptr, 0); }
+  static ByteProvider getConstantZero() { return ByteProvider(nullptr, 0, 0); }
 
   bool isConstantZero() const { return !Load; }
   bool isMemory() const { return Load; }
 
   bool operator==(const ByteProvider &Other) const {
-    return Other.Load == Load && Other.ByteOffset == ByteOffset;
+    return Other.Load == Load && Other.ByteOffset == ByteOffset &&
+           Other.VectorOffset == VectorOffset;
   }
 
 private:
-  ByteProvider(LoadSDNode *Load, unsigned ByteOffset)
-      : Load(Load), ByteOffset(ByteOffset) {}
+  ByteProvider(LoadSDNode *Load, unsigned ByteOffset, unsigned VectorOffset)
+      : Load(Load), ByteOffset(ByteOffset), VectorOffset(VectorOffset) {}
 };
 
 } // end anonymous namespace
 
 /// Recursively traverses the expression calculating the origin of the requested
 /// byte of the given value. Returns None if the provider can't be calculated.
 ///
-/// For all the values except the root of the expression verifies that the value
-/// has exactly one use and if it's not true return None. This way if the origin
-/// of the byte is returned it's guaranteed that the values which contribute to
-/// the byte are not used outside of this expression.
+/// For all the values except the root of the expression, we verify that the
+/// value has exactly one use and if not then return None. This way if the
+/// origin of the byte is returned it's guaranteed that the values which
+/// contribute to the byte are not used outside of this expression.
+
+/// However, there is a special case when dealing with vector loads -- we allow
+/// more than one use if the load is a vector type.  Since the values that
+/// contribute to the byte ultimately come from the ExtractVectorElements of the
+/// Load, we don't care if the Load has uses other than ExtractVectorElements,
+/// because those operations are independent from the pattern to be combined.
+/// For vector loads, we simply care that the ByteProviders are adjacent
+/// positions of the same vector, and their index matches the byte that is being
+/// provided. This is captured by the \p VectorIndex algorithm. \p VectorIndex
+/// is the index used in an ExtractVectorElement, and \p StartingIndex is the
+/// byte position we are trying to provide for the LoadCombine. If these do
+/// not match, then we can not combine the vector loads. \p Index uses the
+/// byte position we are trying to provide for and is matched against the
+/// shl and load size. The \p Index algorithm ensures the requested byte is
+/// provided for by the pattern, and the pattern does not over provide bytes.
 ///
-/// Because the parts of the expression are not allowed to have more than one
-/// use this function iterates over trees, not DAGs. So it never visits the same
-/// node more than once.
+///
+/// The supported LoadCombine pattern for vector loads is as follows
+///                              or
+///                          /        \
+///                         or        shl
+///                       /     \      |
+///                     or      shl   zext
+///                   /    \     |     |
+///                 shl   zext  zext  EVE*
+///                  |     |     |     |
+///                 zext  EVE*  EVE*  LOAD
+///                  |     |     |
+///                 EVE*  LOAD  LOAD
+///                  |
+///                 LOAD
+///
+/// *ExtractVectorElement
 static const Optional<ByteProvider>
 calculateByteProvider(SDValue Op, unsigned Index, unsigned Depth,
-                      bool Root = false) {
+                      Optional<uint64_t> VectorIndex,
+                      unsigned StartingIndex = 0) {
+
   // Typical i64 by i8 pattern requires recursion up to 8 calls depth
   if (Depth == 10)
     return None;
 
-  if (!Root && !Op.hasOneUse())
+  // Multiple uses of vector loads are expected
+  if (Depth && !Op.hasOneUse() &&
+      !(Op.getOpcode() == ISD::LOAD && Op.getValueType().isVector()))
+    return None;
+
+  // Fail to combine if we have encountered anything but a LOAD after handling
+  // an ExtractVectorElement.
+  if (Op.getOpcode() != ISD::LOAD && VectorIndex.has_value())
     return None;
 
-  assert(Op.getValueType().isScalarInteger() && "can't handle other types");
   unsigned BitWidth = Op.getValueSizeInBits();
   if (BitWidth % 8 != 0)
     return None;
   unsigned ByteWidth = BitWidth / 8;
   assert(Index < ByteWidth && "invalid index requested");
   (void) ByteWidth;
 
   switch (Op.getOpcode()) {
   case ISD::OR: {
-    auto LHS = calculateByteProvider(Op->getOperand(0), Index, Depth + 1);
+    auto LHS =
+        calculateByteProvider(Op->getOperand(0), Index, Depth + 1, VectorIndex);
     if (!LHS)
       return None;
-    auto RHS = calculateByteProvider(Op->getOperand(1), Index, Depth + 1);
+    auto RHS =
+        calculateByteProvider(Op->getOperand(1), Index, Depth + 1, VectorIndex);
     if (!RHS)
       return None;
 
     if (LHS->isConstantZero())
       return RHS;
     if (RHS->isConstantZero())
       return LHS;
     return None;
   }
   case ISD::SHL: {
     auto ShiftOp = dyn_cast<ConstantSDNode>(Op->getOperand(1));
     if (!ShiftOp)
       return None;
 
     uint64_t BitShift = ShiftOp->getZExtValue();
+
     if (BitShift % 8 != 0)
       return None;
     uint64_t ByteShift = BitShift / 8;
 
+    // If we are shifting by an amount greater than the index we are trying to
+    // provide, then do not provide anything. Otherwise, subtract the index by
+    // the amount we shifted by.
     return Index < ByteShift
                ? ByteProvider::getConstantZero()
                : calculateByteProvider(Op->getOperand(0), Index - ByteShift,
-                                       Depth + 1);
+                                       Depth + 1, VectorIndex, Index);
   }
   case ISD::ANY_EXTEND:
   case ISD::SIGN_EXTEND:
   case ISD::ZERO_EXTEND: {
     SDValue NarrowOp = Op->getOperand(0);
     unsigned NarrowBitWidth = NarrowOp.getScalarValueSizeInBits();
     if (NarrowBitWidth % 8 != 0)
       return None;
     uint64_t NarrowByteWidth = NarrowBitWidth / 8;
 
     if (Index >= NarrowByteWidth)
       return Op.getOpcode() == ISD::ZERO_EXTEND
                  ? Optional<ByteProvider>(ByteProvider::getConstantZero())
                  : None;
-    return calculateByteProvider(NarrowOp, Index, Depth + 1);
+    return calculateByteProvider(NarrowOp, Index, Depth + 1, VectorIndex,
+                                 StartingIndex);
   }
   case ISD::BSWAP:
     return calculateByteProvider(Op->getOperand(0), ByteWidth - Index - 1,
-                                 Depth + 1);
+                                 Depth + 1, VectorIndex, StartingIndex);
+  case ISD::EXTRACT_VECTOR_ELT: {
+    auto OffsetOp = dyn_cast<ConstantSDNode>(Op->getOperand(1));
+    if (!OffsetOp)
+      return None;
+
+    VectorIndex = OffsetOp->getZExtValue();
+
+    // The byte we are trying to provide (StartingIndex) must correspond with
+    // the vector offset. Otherwise we are shuffling the elements in a vector
+    // and bitpicking them into a scalar, and such a pattern should not be
+    // combined into a load.
+    if (StartingIndex != OffsetOp->getZExtValue())
+      return None;
+
+    return calculateByteProvider(Op->getOperand(0), Index, Depth + 1,
+                                 VectorIndex, StartingIndex);
+  }
   case ISD::LOAD: {
     auto L = cast<LoadSDNode>(Op.getNode());
     if (!L->isSimple() || L->isIndexed())
       return None;
 
     unsigned NarrowBitWidth = L->getMemoryVT().getSizeInBits();
     if (NarrowBitWidth % 8 != 0)
       return None;
     uint64_t NarrowByteWidth = NarrowBitWidth / 8;
 
+    // If the width of the load does not reach byte we are trying to provide for
+    // and it is not a ZEXTLOAD, then the load does not provide for the byte in
+    // question
     if (Index >= NarrowByteWidth)
       return L->getExtensionType() == ISD::ZEXTLOAD
                  ? Optional<ByteProvider>(ByteProvider::getConstantZero())
                  : None;
-    return ByteProvider::getMemory(L, Index);
+
+    unsigned BPVectorIndex = VectorIndex.value_or(0U);
+    return ByteProvider::getMemory(L, Index, BPVectorIndex);
   }
   }
 
   return None;
 }
 
 static unsigned littleEndianByteAt(unsigned BW, unsigned i) {
   return i;
SDValue DAGCombiner::MatchLoadCombine(SDNode *N) {
   EVT VT = N->getValueType(0);
   if (VT != MVT::i16 && VT != MVT::i32 && VT != MVT::i64)
     return SDValue();
   unsigned ByteWidth = VT.getSizeInBits() / 8;
 
   bool IsBigEndianTarget = DAG.getDataLayout().isBigEndian();
   auto MemoryByteOffset = [&] (ByteProvider P) {
     assert(P.isMemory() && "Must be a memory byte provider");
-    unsigned LoadBitWidth = P.Load->getMemoryVT().getSizeInBits();
+    unsigned LoadBitWidth = P.Load->getMemoryVT().getScalarSizeInBits();
+
     assert(LoadBitWidth % 8 == 0 &&
            "can only analyze providers for individual bytes not bit");
     unsigned LoadByteWidth = LoadBitWidth / 8;
     return IsBigEndianTarget
             ? bigEndianByteAt(LoadByteWidth, P.ByteOffset)
             : littleEndianByteAt(LoadByteWidth, P.ByteOffset);
   };
 
   Optional<BaseIndexOffset> Base;
   SDValue Chain;
 
   SmallPtrSet<LoadSDNode *, 8> Loads;
   Optional<ByteProvider> FirstByteProvider;
   int64_t FirstOffset = INT64_MAX;
 
   // Check if all the bytes of the OR we are looking at are loaded from the same
   // base address. Collect bytes offsets from Base address in ByteOffsets.
   SmallVector<int64_t, 8> ByteOffsets(ByteWidth);
   unsigned ZeroExtendedBytes = 0;
   for (int i = ByteWidth - 1; i >= 0; --i) {
-    auto P = calculateByteProvider(SDValue(N, 0), i, 0, /*Root=*/true);
+    auto P =
+        calculateByteProvider(SDValue(N, 0), i, 0, /*VectorIndex*/ None, i);
     if (!P)
       return SDValue();
 
     if (P->isConstantZero()) {
       // It's OK for the N most significant bytes to be 0, we can just
       // zero-extend the load.
       if (++ZeroExtendedBytes != (ByteWidth - static_cast<unsigned>(i)))
         return SDValue();
       continue;
     }
     assert(P->isMemory() && "provenance should either be memory or zero");
 
     LoadSDNode *L = P->Load;
-    assert(L->hasNUsesOfValue(1, 0) && L->isSimple() &&
-           !L->isIndexed() &&
-           "Must be enforced by calculateByteProvider");
-    assert(L->getOffset().isUndef() && "Unindexed load must have undef offset");
 
     // All loads must share the same chain
     SDValue LChain = L->getChain();
     if (!Chain)
       Chain = LChain;
     else if (Chain != LChain)
       return SDValue();
 
     // Loads must share the same base address
     BaseIndexOffset Ptr = BaseIndexOffset::match(L, DAG);
     int64_t ByteOffsetFromBase = 0;
+
+    // Add the VectorOffset (if any) to the offset of Ptr
+    Ptr.addToOffset(P->VectorOffset);
+
     if (!Base)
       Base = Ptr;
+
     else if (!Base->equalBaseIndex(Ptr, DAG, ByteOffsetFromBase))
       return SDValue();
 
     // Calculate the offset of the current byte from the base address
     ByteOffsetFromBase += MemoryByteOffset(*P);
     ByteOffsets[i] = ByteOffsetFromBase;
 
     // Remember the first byte load

llvm/test/CodeGen/AArch64/load-combine.ll

; CHECK-NEXT:    ret
   %tmp4 = getelementptr inbounds i8, i8* %tmp, i32 0
   %tmp5 = load i8, i8* %tmp4, align 2
   %tmp6 = zext i8 %tmp5 to i32
   %tmp7 = shl nuw nsw i32 %tmp6, 24
   %tmp8 = or i32 %tmp7, %tmp30
   ret i32 %tmp8
 }
 
+; x1 = x0
 define void @short_vector_to_i32(<4 x i8>* %in, i32* %out, i32* %p) {
 ; CHECK-LABEL: short_vector_to_i32:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    ldr s0, [x0]
-; CHECK-NEXT:    ushll v0.8h, v0.8b, #0
-; CHECK-NEXT:    umov w8, v0.h[0]
-; CHECK-NEXT:    umov w9, v0.h[1]
-; CHECK-NEXT:    umov w10, v0.h[2]
-; CHECK-NEXT:    umov w11, v0.h[3]
-; CHECK-NEXT:    bfi w8, w9, #8, #8
-; CHECK-NEXT:    bfi w8, w10, #16, #8
-; CHECK-NEXT:    bfi w8, w11, #24, #8
+; CHECK-NEXT:    ldr w8, [x0]
 ; CHECK-NEXT:    str w8, [x1]
 ; CHECK-NEXT:    ret
   %ld = load <4 x i8>, <4 x i8>* %in, align 4
 
   %e1 = extractelement <4 x i8> %ld, i32 0
   %e2 = extractelement <4 x i8> %ld, i32 1
   %e3 = extractelement <4 x i8> %ld, i32 2
   %e4 = extractelement <4 x i8> %ld, i32 3
; CHECK-NEXT:    ret
   store i32 %i3, i32* %out
   ret void
 }
 
 define void @short_vector_to_i32_unused_high_i8(<4 x i8>* %in, i32* %out, i32* %p) {
 ; CHECK-LABEL: short_vector_to_i32_unused_high_i8:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ldr s0, [x0]
+; CHECK-NEXT:    ldrh w9, [x0]
 ; CHECK-NEXT:    ushll v0.8h, v0.8b, #0
-; CHECK-NEXT:    umov w8, v0.h[0]
-; CHECK-NEXT:    umov w9, v0.h[1]
-; CHECK-NEXT:    umov w10, v0.h[2]
-; CHECK-NEXT:    bfi w8, w9, #8, #8
-; CHECK-NEXT:    bfi w8, w10, #16, #8
-; CHECK-NEXT:    str w8, [x1]
+; CHECK-NEXT:    umov w8, v0.h[2]
+; CHECK-NEXT:    bfi w9, w8, #16, #8
+; CHECK-NEXT:    str w9, [x1]
 ; CHECK-NEXT:    ret
   %ld = load <4 x i8>, <4 x i8>* %in, align 4
 
   %e1 = extractelement <4 x i8> %ld, i32 0
   %e2 = extractelement <4 x i8> %ld, i32 1
   %e3 = extractelement <4 x i8> %ld, i32 2
 
   %z0 = zext i8 %e1 to i32
; CHECK-NEXT:    ret
   %s3 = shl nuw i32 %z3, 24
 
   %i3 = or i32 %s2, %s3
 
   store i32 %i3, i32* %out
   ret void
 }
 
+; x1 = x0[0:1]
 define void @short_vector_to_i32_unused_high_i16(<4 x i8>* %in, i32* %out, i32* %p) {
 ; CHECK-LABEL: short_vector_to_i32_unused_high_i16:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    ldr s0, [x0]
-; CHECK-NEXT:    ushll v0.8h, v0.8b, #0
-; CHECK-NEXT:    umov w8, v0.h[0]
-; CHECK-NEXT:    umov w9, v0.h[1]
-; CHECK-NEXT:    bfi w8, w9, #8, #8
+; CHECK-NEXT:    ldrh w8, [x0]
 ; CHECK-NEXT:    str w8, [x1]
 ; CHECK-NEXT:    ret
   %ld = load <4 x i8>, <4 x i8>* %in, align 4
 
   %e1 = extractelement <4 x i8> %ld, i32 0
   %e2 = extractelement <4 x i8> %ld, i32 1
 
   %z0 = zext i8 %e1 to i32
   %z1 = zext i8 %e2 to i32
 
   %s1 = shl nuw nsw i32 %z1, 8
 
   %i1 = or i32 %s1, %z0
 
   store i32 %i1, i32* %out
   ret void
 }
 
+; x1 = x0
 define void @short_vector_to_i64(<4 x i8>* %in, i64* %out, i64* %p) {
 ; CHECK-LABEL: short_vector_to_i64:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    ldr s0, [x0]
-; CHECK-NEXT:    ushll v0.8h, v0.8b, #0
-; CHECK-NEXT:    umov w8, v0.h[0]
-; CHECK-NEXT:    umov w9, v0.h[1]
-; CHECK-NEXT:    umov w10, v0.h[2]
-; CHECK-NEXT:    umov w11, v0.h[3]
-; CHECK-NEXT:    bfi x8, x9, #8, #8
-; CHECK-NEXT:    bfi x8, x10, #16, #8
-; CHECK-NEXT:    bfi x8, x11, #24, #8
+; CHECK-NEXT:    ldr w8, [x0]
 ; CHECK-NEXT:    str x8, [x1]
 ; CHECK-NEXT:    ret
   %ld = load <4 x i8>, <4 x i8>* %in, align 4
 
   %e1 = extractelement <4 x i8> %ld, i32 0
   %e2 = extractelement <4 x i8> %ld, i32 1
   %e3 = extractelement <4 x i8> %ld, i32 2
   %e4 = extractelement <4 x i8> %ld, i32 3

llvm/test/CodeGen/AMDGPU/combine-vload-extract.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
 
 define amdgpu_kernel void @vectorLoadCombine(<4 x i8>* %in, i32* %out) {
 ; GCN-LABEL: vectorLoadCombine:
 ; GCN:       ; %bb.0: ; %entry
 ; GCN-NEXT:    s_load_dwordx4 s[0:3], s[0:1], 0x24
 ; GCN-NEXT:    s_waitcnt lgkmcnt(0)
 ; GCN-NEXT:    v_mov_b32_e32 v0, s0
 ; GCN-NEXT:    v_mov_b32_e32 v1, s1
 ; GCN-NEXT:    flat_load_dword v2, v[0:1]
-; GCN-NEXT:    s_mov_b32 s0, 0x6050400
 ; GCN-NEXT:    v_mov_b32_e32 v0, s2
 ; GCN-NEXT:    v_mov_b32_e32 v1, s3
 ; GCN-NEXT:    s_waitcnt vmcnt(0) lgkmcnt(0)
-; GCN-NEXT:    v_bfe_u32 v3, v2, 8, 8
-; GCN-NEXT:    v_and_b32_e32 v4, 0xff0000, v2
-; GCN-NEXT:    v_perm_b32 v3, v3, v2, s0
-; GCN-NEXT:    v_and_b32_e32 v2, 0xff000000, v2
-; GCN-NEXT:    v_or3_b32 v2, v3, v4, v2
 ; GCN-NEXT:    flat_store_dword v[0:1], v2
 ; GCN-NEXT:    s_endpgm
 entry:
   %0 = load <4 x i8>, <4 x i8>* %in, align 4
   %1 = extractelement <4 x i8> %0, i32 0
   %2 = extractelement <4 x i8> %0, i32 1
   %3 = extractelement <4 x i8> %0, i32 2
   %4 = extractelement <4 x i8> %0, i32 3
entry:
   %zext2 = zext i8 %2 to i32
   %shift2 = shl nuw nsw i32 %zext2, 16
   %insert2 = or i32 %insert1, %shift2
   %zext3 = zext i8 %4 to i32
   %shift3 = shl nuw i32 %zext3, 24
   %insert3 = or i32 %insert2, %shift3
   store i32 %insert3, i32* %out
   ret void
 }