[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 ↗

(On Diff #197001)

Please clang-format the function

6132 ↗

(On Diff #197001)

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

[steven.zhang]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

6128 ↗

(On Diff #197001)

oops. copy paste issue when resolve the conflict.

6132 ↗

(On Diff #197001)

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 ↗

(On Diff #197050)

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 ↗

(On Diff #197050)

Or maybe std::optional?

[steven.zhang]

Address comments.

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

6245 ↗

(On Diff #197260)

Maybe call this IsBigEndian?

[steven.zhang]

Update local variable name.

[RKSimon]

LGTM - with one minor

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

6246 ↗

(On Diff #198166)

Maybe if (!IsBigEndian.hasValue()) ?

[steven.zhang]

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

llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (Index >= NarrowByteWidth)
                  : None;
     return ByteProvider::getMemory(L, Index);
   }
   }
 
   return None;
 }
 
+static unsigned LittleEndianByteAt(unsigned BW, unsigned i) {
+  return i;
+}
+
+static unsigned BigEndianByteAt(unsigned BW, unsigned i) {
+  return BW - i - 1;
+}
+
+// Check if the bytes offsets we are looking at match with either big or
+// little endian value loaded. Return true for big endian, false for little 
+// endian, and None if match failed.
+static Optional<bool> isBigEndian(const SmallVector<int64_t, 4> &ByteOffsets,
+                                  int64_t FirstOffset) {
+  // The endian can be decided only when it is 2 bytes at least.
+  unsigned Width = ByteOffsets.size();
+  if (Width < 2)
+    return None;
+
+  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 None;
+  }
+
+  assert((BigEndian != LittleEndian) && "It should be either big endian or"
+                                        "little endian");
+  return BigEndian;
+}
+
 /// 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();
 
-  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
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");
+  Optional<bool> IsBigEndian = isBigEndian(ByteOffsets, FirstOffset);
+  if (!IsBigEndian.hasValue())
+
   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;
 
   // The node we are looking at matches with the pattern, check if we can
   // replace it with a single load and bswap if needed.
 
   // If the load needs byte swap check if the target supports it
-  bool NeedsBswap = IsBigEndianTarget != BigEndian;
+  bool NeedsBswap = IsBigEndianTarget != *IsBigEndian;
 
   // Before legalize we can introduce illegal bswaps which will be later
   // converted to an explicit bswap sequence. This way we end up with a single
   // load and byte shuffling instead of several loads and byte shuffling.
   if (NeedsBswap && LegalOperations && !TLI.isOperationLegal(ISD::BSWAP, VT))
     return SDValue();
 
   // Check that a load of the wide type is both allowed and fast on the target