[PATCH] [SelectionDAG] Combine extload/store/load sequence into extload.

Ahmed Bougacha ahmed.bougacha at gmail.com
Wed Jan 21 12:02:19 PST 2015


Before committing, a few more questions, inline.

Thanks for the review!
-Ahmed


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8314
@@ +8313,3 @@
+  //   store i32 %value, %ptr2
+  //     %value = load i32 %ptr1
+  // to turn the extload into:
----------------
qcolombet wrote:
> IIRC, we tend to write the chain of computation in the opposite order:
> load
> store
> extload
> 
> Please make sure you use the same convention here and I did not remember correctly, then ignore this :).
You're right, I was corrupted by SDNode::dumpr() :P

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8317
@@ +8316,3 @@
+  // v4i32 load %ptr1, zext from v4i8
+  if (!ISD::isNON_EXTLoad(N) && !LD->isVolatile() &&
+      ISD::isNON_TRUNCStore(Chain.getNode())) {
----------------
qcolombet wrote:
> !ISD::isNON_EXTLoad(N) => ISD::isEXTLoad(N)
> That may be less contrived.
The name is misleading: isEXTLoad checks for "anyext-loads", not "any kind of extload"!

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8323
@@ +8322,3 @@
+    EVT MemVT = LD->getMemoryVT();
+    if (PrevST->getBasePtr() == Ptr &&
+        (ValVT.getStoreSize() == MemVT.getStoreSize() &&
----------------
Should we also check that the types are equivalent?  For instance, if you have 

   %v = load float %p1
   store float %v, %p2
   %v2 = i64 load %p2, zext from i32

would it be a problem? (I don't think so, but I'm having second thoughts.)

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8329
@@ +8328,3 @@
+      if (LD->getAddressSpace() == PPrevLD->getAddressSpace() &&
+          LD->getAlignment() == PPrevLD->getAlignment()) {
+        SDValue NewLoad = DAG.getExtLoad(
----------------
I think we can remove this check: we don't really care that the alignment is the same, do we?  As long as the new load has the same alignment as the first (non-extending) one.

http://reviews.llvm.org/D6552

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list