[PATCH] D43245: [LoopInterchange] Support reductions across inner and outer loop.
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 31 12:26:05 PDT 2018
fhahn added a comment.
Thanks Eli!
================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:829
- // Outer loop cannot have reduction because then loops will not be tightly
- // nested.
- if (!Reductions.empty()) {
----------------
efriedma wrote:
> Given you're deleting this, is there some check that disallows interchanging if a PHI is a reduction in the outer loop, but not the inner loop? Something like:
>
> ```
> for( int i=1;i<N;i++) {
> for( int j=1;j<N;j++) {
> A[i][j]++;
> }
> X += j;
> }
> ```
Right! We should keep the check and could just not add PHIs across inner and outer loop there I think
================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:1521
+ if (InnerLoop->contains(InBB) ||
+ isa<PHINode>(PHI->getIncomingValueForBlock(InBB)))
continue;
----------------
efriedma wrote:
> Checking `isa<PHINode>()` here seems sort of confusing... is this supposed to be checking for a PHI node from the outer loop? If so, is there some guarantee it isn't a PHI node from somewhere else?
Yeah, it seems like inner loop reductions are not handled properly to start with. I will look into that first. It might be helpful to keep track of reduction/induction phis after the initial checks and use that info here (and other places, rather than recomputing)
https://reviews.llvm.org/D43245
More information about the llvm-commits
mailing list