[NFC] Add a static function to do the endian check

[steven.zhang]

Summary

Add a new function to do the endian check, as I will commit another patch later, which will also need the endian check. The coming patch is trying to combine several stores, as what MatchLoadCombine did. The opportunity is found from spec2017.

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

6128

Please clang-format the function

6132

Anyway that we can avoid duplicate copies of LittleEndianByteAt + BigEndianByteAt?

[steven.zhang]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

6128

oops. copy paste issue when resolve the conflict.

6132

Maybe, remove the LittleEndianByteAt + BigEndianByteAt for load combine ? As it is a really simple helper function, and it is only used in one place now

auto MemoryByteOffset = [&] (ByteProvider P) {
    assert(P.isMemory() && "Must be a memory byte provider");
    unsigned LoadBitWidth = P.Load->getMemoryVT().getSizeInBits();
    assert(LoadBitWidth % 8 == 0 &&
           "can only analyze providers for individual bytes not bit");
    unsigned LoadByteWidth = LoadBitWidth / 8;
    return IsBigEndianTarget ? LoadByteWidth - P.ByteOffset - 1 : P.ByteOffset;
  };

[steven.zhang]

Update the patch according to the reviewer comments.

[apilipenko]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

6135

Small stylistic suggestion. This function essentially returns a tri-state: big, little, neither. An explicit enum for the return value might be a cleaner way to express this.

[nemanjai]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

6135

Or maybe std::optional?

[steven.zhang]

Address comments.

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

6272–6275

Maybe call this IsBigEndian?

[steven.zhang]

Update local variable name.

[RKSimon]

LGTM - with one minor

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

6273

Maybe if (!IsBigEndian.hasValue()) ?

[steven.zhang]

ok. Use the hasValue instead of the implicit bool convert operator to make the code clear.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (Index >= NarrowByteWidth)
                  : None;
     return ByteProvider::getMemory(L, Index);
   }
   }
 
   return None;
 }
 
+// Check if the bytes offsets we are looking at match with either big or
+// little endian value loaded. If it returns true, the endian is set in
+// IsBigEndian, otherwise, the match failed.
+static bool matchEndian(const SmallVector<int64_t, 4> &ByteOffsets,
+                          int64_t FirstOffset, bool &IsBigEndian) {
+  std::function<unsigned(unsigned, unsigned)> LittleEndianByteAt = [](
+    unsigned BW, unsigned i) { return i; };
+  std::function<unsigned(unsigned, unsigned)> BigEndianByteAt = [](
+    unsigned BW, unsigned i) { return BW - i - 1; };
+
+  // The endian can be decided only when it is 2 bytes at least.
+  unsigned Width = ByteOffsets.size();
+  if (Width < 2)
+    return false;
+
+  bool BigEndian = true, LittleEndian = true;
+  for (unsigned i = 0; i < Width; i++) {
+    int64_t CurrentByteOffset = ByteOffsets[i] - FirstOffset;
+    LittleEndian &= CurrentByteOffset == LittleEndianByteAt(Width, i);
+    BigEndian &= CurrentByteOffset == BigEndianByteAt(Width, i);
+    if (!BigEndian && !LittleEndian)
+      return false;
+  }
+
+  assert((BigEndian != LittleEndian) && "It should be either big endian or"
+                                        "little endian");
+  IsBigEndian = BigEndian;
+  return true;
+}
+
 /// Match a pattern where a wide type scalar value is loaded by several narrow
 /// loads and combined by shifts and ors. Fold it into a single load or a load
 /// and a BSWAP if the targets supports it.
 ///
 /// Assuming little endian target:
 ///  i8 *a = ...
 ///  i32 val = a[0] | (a[1] << 8) | (a[2] << 16) | (a[3] << 24)
 /// =>
SDValue DAGCombiner::MatchLoadCombine(SDNode *N) {
 
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
   // Before legalize we can introduce too wide illegal loads which will be later
   // split into legal sized loads. This enables us to combine i64 load by i8
   // patterns to a couple of i32 loads on 32 bit targets.
   if (LegalOperations && !TLI.isOperationLegal(ISD::LOAD, VT))
     return SDValue();
 
+  bool IsBigEndianTarget = DAG.getDataLayout().isBigEndian();
+  auto MemoryByteOffset = [&] (ByteProvider P) {
     std::function<unsigned(unsigned, unsigned)> LittleEndianByteAt = [](
       unsigned BW, unsigned i) { return i; };
     std::function<unsigned(unsigned, unsigned)> BigEndianByteAt = [](
       unsigned BW, unsigned i) { return BW - i - 1; };
 
-  bool IsBigEndianTarget = DAG.getDataLayout().isBigEndian();
-  auto MemoryByteOffset = [&] (ByteProvider P) {
     assert(P.isMemory() && "Must be a memory byte provider");
     unsigned LoadBitWidth = P.Load->getMemoryVT().getSizeInBits();
     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);
SDValue DAGCombiner::MatchLoadCombine(SDNode *N) {
   }
   assert(!Loads.empty() && "All the bytes of the value must be loaded from "
          "memory, so there must be at least one load which produces the value");
   assert(Base && "Base address of the accessed memory location must be set");
   assert(FirstOffset != INT64_MAX && "First byte offset must be set");
 
   // Check if the bytes of the OR we are looking at match with either big or
   // little endian value load
-  bool BigEndian = true, LittleEndian = true;
-  for (unsigned i = 0; i < ByteWidth; i++) {
-    int64_t CurrentByteOffset = ByteOffsets[i] - FirstOffset;
-    LittleEndian &= CurrentByteOffset == LittleEndianByteAt(ByteWidth, i);
-    BigEndian &= CurrentByteOffset == BigEndianByteAt(ByteWidth, i);
-    if (!BigEndian && !LittleEndian)
-      return SDValue();
-  }
-  assert((BigEndian != LittleEndian) && "should be either or");
+  bool BigEndian = false;
+  if (!matchEndian(ByteOffsets, FirstOffset, BigEndian))
+
   assert(FirstByteProvider && "must be set");
 
   // Ensure that the first byte is loaded from zero offset of the first load.
   // So the combined value can be loaded from the first load address.
   if (MemoryByteOffset(*FirstByteProvider) != 0)
     return SDValue();
   LoadSDNode *FirstLoad = FirstByteProvider->Load;