This is an archive of the discontinued LLVM Phabricator instance.

[IndVars] Simplify instructions after replacing header phi with preheader value
ClosedPublic

Authored by nikic on Jul 7 2022, 8:10 AM.

Details

Summary

After replacing a loop phi with the preheader value, it's usually possible to simplify some of the using instructions, so do that as part of replaceLoopPHINodesWithPreheaderValues().

Doing this as part of IndVars is valuable, because it may make GEPs in the loop have constant offsets and allow the following SROA run to succeed (as demonstrated in the PhaseOrdering test).

Depends on D129214.

Diff Detail

Event Timeline

nikic created this revision.Jul 7 2022, 8:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 7 2022, 8:10 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
nikic requested review of this revision.Jul 7 2022, 8:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 7 2022, 8:10 AM
nikic updated this revision to Diff 443198.Jul 8 2022, 4:07 AM
nikic edited the summary of this revision. (Show Details)

Add PhaseOrdering test.

reames accepted this revision.Jul 8 2022, 10:18 AM

LGTM

This revision is now accepted and ready to land.Jul 8 2022, 10:18 AM
This revision was landed with ongoing or failed builds.Jul 13 2022, 1:27 AM
This revision was automatically updated to reflect the committed changes.

This triggers failed asserts for me, with https://martin.st/temp/macroblock-reduced.c:

$ clang -target aarch64-linux-gnu -c macroblock-reduced.c -O2
clang: ../lib/Transforms/Scalar/IndVarSimplify.cpp:2059: bool {anonymous}::IndVarSimplify::run(llvm::Loop*): Assertion `L->isRecursivelyLCSSAForm(*DT, *LI) && "Indvars did not preserve LCSSA!"' failed.
nikic added a comment.Jul 14 2022, 2:27 AM

@mstorsjo Thanks for the report! Here's a reduced test case:

define i64 @test(i64 %arg) {
entry:
  br label %loop1

loop1:
  %iv1 = phi i64 [ 0, %entry ], [ 1, %loop1.latch ]
  br label %loop2

loop2:
  %iv2 = phi i64 [ 0, %loop1 ], [ 1, %loop2 ]
  %res = add nuw nsw i64 %iv1, %iv2
  br i1 true, label %loop2, label %loop1.latch

loop1.latch:
  %res.lcssa = phi i64 [ %res, %loop2 ]
  br i1 false, label %loop1, label %exit

exit:
  %res.lcssa2 = phi i64 [ %res.lcssa, %loop1.latch ]
  ret i64 %res.lcssa2
}

This gets reduced to:

define i64 @t(i64 %arg) {
entry:
  br label %loop1

loop1:                                            ; preds = %loop1.latch, %entry
  br label %loop2

loop2:                                            ; preds = %loop2, %loop1
  %iv2 = phi i64 [ 0, %loop1 ], [ 1, %loop2 ]
  br i1 true, label %loop2, label %loop1.latch

loop1.latch:                                      ; preds = %loop2
  br i1 false, label %loop1, label %exit

exit:                                             ; preds = %loop1.latch
  %res.lcssa2 = phi i64 [ %iv2, %loop1.latch ]
  ret i64 %res.lcssa2
}

We need to make sure that we don't fold away any LCSSA phi nodes of inner loops.

Thanks! The issue does indeed seem to be fixed. (Sorry for the long delay in properly rechecking the issue.)