[PATCH] Fix CombineToPostIndexedLoadStore in DAGCombiner.cpp

[Quentin Colombet]

Hi François,

Since you are fixing two problems, I would prefer having two different patches/commits.

The fix for #1 looks good to me.
Please add a test case for it and commit it separately.

The fix for #2 is almost good. You just need the change "TryNext to RealUse" and the removal of the ADD/SUB check. The outer loop must not be removed.
Please add your test case to the patch (not just in the comment) so that it runs with make check and upload the new patch.

Thanks,
-Quentin


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8853
@@ -8852,4 +8852,3 @@
   for (SDNode *Op : Ptr.getNode()->uses()) {
-    if (Op == N ||
-        (Op->getOpcode() != ISD::ADD && Op->getOpcode() != ISD::SUB))
+    if (Op->getOpcode() != ISD::ADD && Op->getOpcode() != ISD::SUB)
       continue;
----------------
I don’t get why this change is useful.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8878
@@ +8877,3 @@
+      // that cannot be folded as addressing mode
+      // If one use is not a load / store address, then do the transformation.
+      bool RealUse = false;
----------------
This change looks wrong to me.
You basically assume that BasePtr == Ptr, which AFAICT, is not necessarily true.

To me, the proper fix would be to simply replace TryNext by RealUse and use the opposite logic, like you did.

http://reviews.llvm.org/D9107

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