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

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

May I suggest we only test what this patch actually changes? This patch adds an option which when enabled allows interleaving of loops with small trip counts and scalar reductions, so it suffices to test exactly that. That should be covered by llvm/test/Transforms/LoopVectorize/PowerPC/interleave_IC.ll. I think the other test cases can be removed. IMHO adding more tests to make sure SLP vectorization happens (and the like) are redundant, add unnecessary maintenance in the future and are beyond the scope of what this patch is trying to do.



================
Comment at: llvm/test/Transforms/LoopVectorize/PowerPC/interleave_IC.ll:1
+; RUN: opt < %s -loop-vectorize -mtriple=powerpc64le-unknown-linux -S -mcpu=pwr9 -interleave-small-loop-scalar-reduction=true 2>&1 | FileCheck %s
+; RUN: opt < %s -passes='loop-vectorize' -mtriple=powerpc64le-unknown-linux -S -mcpu=pwr9 -interleave-small-loop-scalar-reduction=true 2>&1 | FileCheck %s
----------------
There is a `target triple` in the IR too so `-mtriple=powerpc64le-unknown-linux ` should not be necessary. Alternatively you can remove the triple from the IR if that's what the other test cases in this directory do.


================
Comment at: llvm/test/Transforms/PhaseOrdering/interleave_LV_SLP.ll:3
+; REQUIRES: powerpc-registered-target
+; RUN: opt < %s -O2 -mtriple=powerpc64le-unknown-linux -S -mcpu=pwr9 -interleave-small-loop-scalar-reduction=true 2>&1 | FileCheck %s
+; RUN: opt < %s -passes='default<O2>' -mtriple=powerpc64le-unknown-linux -S -mcpu=pwr9 -interleave-small-loop-scalar-reduction=true 2>&1 | FileCheck %s
----------------
please see my comment about triple.


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

https://reviews.llvm.org/D81416