[PATCH] D27861: [DAGCombiner] Match load by bytes idiom and fold it into a single load. Attempt #2.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

A few minors - anyone else have any comments?



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4503
+    assert(L->hasNUsesOfValue(1, 0) && !L->isVolatile() && !L->isIndexed() &&
+           (L->getExtensionType() == ISD::NON_EXTLOAD) &&
+           "Must be enforced by calculateByteProvider");
----------------
Are there any situations where we can use ZEXTLOAD? If so add a TODO comment?


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4511
+      Chain = LChain;
+    if (Chain != LChain)
+      return SDValue();
----------------
Avoid a comparison we know the result of:
```
else if (Chain != LChain)
```


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4518
+      Base = Ptr;
+    if (!Base->equalBaseIndex(Ptr))
+      return SDValue();
----------------
Avoid a comparison we know the result of:
```
else if (!Base->equalBaseIndex(Ptr))
```


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4527
+    int64_t MemoryByteOffset =
+        DAG.getDataLayout().isBigEndian()
+            ? BigEndianByteAt(LoadByteWidth, P->ByteOffset)
----------------
Loop invariant - pull this out to the top of the function:
```
bool IsBigEndianTarget = DAG.getDataLayout().isBigEndian();
```


================
Comment at: test/CodeGen/X86/load-combine.ll:654
+; CHECK64-NEXT:    retq
+; Currently we don't fold the pattern for x86-64 target because we don't see
+; that the loads are adjacent. It happens because BaseIndexOffset doesn't look
----------------
Add a TODO to the comment


https://reviews.llvm.org/D27861