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

Karthik Bhat kv.bhat at samsung.com
Tue Apr 21 05:38:50 PDT 2015


================
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"
+
----------------
rengolin wrote:
> karthikthecool wrote:
> > rengolin wrote:
> > > 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.
> > Wont this only run if triple is x86_64-unknown-linux-gnu? But i agree i need to add some general tests as well. 
> No. To force it to run only on one architecture is to either REQUIRE: x86_64 or to move it into a directory that is platform-specific, with a lit.cfg file that does the same thing. But we want neither.
> 
> Since this is not a platform pass, I think this optimisation should be tested in all platforms, unless we have a good reason not to. Taking away the triple will ensure that the target will be picked as the host, which is what we want. The IR might have to change to accommodate on other targets. I can test it on ARM/AArch64 to be sure.
Interesting because i had added some testcases with target triple in the initial checkin of this pass and they seems to pass. 
Also i saw similar tests in LoopReroll which i took as reference. 

But i agree we need generic tests. I will add/update the same.
Wow it would be great if you could help me with the results of ARM/AArch64. I will try out the same as well.

================
Comment at: test/Transforms/LoopInterchange/reductions.ll:124
@@ +123,3 @@
+
+; CHECK-LABEL: @reduction_02
+; CHECK:  for.body6:                                        ; preds = %for.body6.preheader, %for.body6.split
----------------
rengolin wrote:
> karthikthecool wrote:
> > rengolin wrote:
> > > 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.
> > The problem here was I wanted to check if loops are interchanged properly and for that i was checking the complete structure of interchanged loop for correctness. I will try to reduce the checks.
> That's fine, but you can't rely on the types and names as much as in the sequence of instructions. Only parse types and variables if they *must* be of a specific pattern. If not, just check the instruction names and correct order.
Got your point. Will update the tests by tomorrow.
Thanks for the comments.:)

http://reviews.llvm.org/D8314

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






More information about the llvm-commits mailing list