[PATCH] [LoopInterchange] Add support to interchange loops with reductions.

[Renato Golin]

Hi Karthik,

The patch is looking good, apart from the few comments. I'd welcome @jmolloy's comments.

cheers,
--renato


================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:611
@@ -592,1 +610,3 @@
+        return true;
+    } else {
     if (I->mayHaveSideEffects() || I->mayReadFromMemory())
----------------
nitpick, please join this "else if".

================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:628
@@ +627,3 @@
+        return true;
+    } else {
+      if (I->mayHaveSideEffects() || I->mayReadFromMemory())
----------------
nitpick, please join this "else if".

================
Comment at: test/Transforms/LoopInterchange/reductions.ll:3
@@ +2,3 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
----------------
This is a generic test, it must not contain a target triple or it will fail on all aches minus x86_64. We do want to test this in ARM, MIPS, PPC, etc, so we should remove the triple and make sure it works on all buildbots.

================
Comment at: test/Transforms/LoopInterchange/reductions.ll:124
@@ +123,3 @@
+
+; CHECK-LABEL: @reduction_02
+; CHECK:  for.body6:                                        ; preds = %for.body6.preheader, %for.body6.split
----------------
These check lines are bound to fail on multiple architectures and with other optimisations coming in later, they could break the sequence or reorder  instructions. You need to parse what's really relevant in the right order with the right arguments stored in variables "[[foo]]" and removed of architecture types to make sure it passes on 16/32/64 bit machines.

The same is true for the other tests.

http://reviews.llvm.org/D8314

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