[PATCH] D81416: [LV] Interleave to expose ILP for small loops with scalar reductions.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/test/Transforms/LoopVectorize/PowerPC/interleave_IC.ll:55
+%14 = type { %15, i64 }
+%15 = type { i32, %15*, %15*, %15* }
+%16 = type { i32 (...)**, i8* }
----------------
AaronLiu wrote:
> fhahn wrote:
> > AaronLiu wrote:
> > > fhahn wrote:
> > > > Area all those types necessary? Would be good to clean up the test, including the GEPs with null/undef, otherwise the test might be painful to update in the future.
> > > The testcase is extracted and reduced from a real application which has very complex and nested data structures. This is the reason why it has those types defined in the IR.
> > sure, unfortunately the reduction tools are not perfect. Still, ideally the test would be as small as necessary to illustrate the problem. Otherwise it will potentially become a burden when making further changes. There are only a few memory accesses in the test and it should be possible to update them to use regular types. 
> Agree, the reduction tools are not perfect. We tried very hard to reduce the testcase, and this is probably the smallest testcase that we can reduce to. It is not easy to come up with a testcase purely from imagination that use only a few memory accesses which can satisfy constraints such as should be able to legally vectorized by LV and at the same time too expensive to be vectorized and refused by the cost model, and the trip count should be compile time unknown and relatively small in run time.
I was not suggesting to come up with a test case from scratch, I was suggesting to inspect the reduced test case to check if there are unnecessary bits, like the types.

If you look at the defined types, they are only used in the entry block and it should be possible to replace them with something like

```
define dso_local void @test(i32*** %arg, double** %arg1) align 2 {
bb:
  %tpm15 = load i32**, i32*** %arg, align 8
  %tpm19 = load double*, double** %arg1, align 8
  br label %bb22
```

Similarly, instead of using `null` as base pointer for `GEPs` and `undef` as index, it should be possible to use either a global or a pointe argument as base and remove the UB from the test case. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81416/new/

https://reviews.llvm.org/D81416