This is an archive of the discontinued LLVM Phabricator instance.

[LoopPredication] Insert assumes of conditions of predicated guards
ClosedPublic

Authored by dmakogon on Oct 6 2022, 3:53 AM.

Details

Summary

As LoopPredication performs non-equivalent transforms removing some checks from loops, other passes may not be able to perform transforms they'd be able to do if the checks were left in loops.
For example, LoopPredication may turn a range check expressed via widenable condition like this

loop:
%iv.1 = phi i32 [ %iv.1.start, %entry ], [ %iv.1.next, %latch ]
%cond.1 = icmp ult i32 %iv.1, %iv.1.end
%wc = call i1 @llvm.experimental.widenable.condition()
%explicit_guard_cond = and i1 %cond.1, %wc
br i1 %explicit_guard_cond, label %loop.next, label %deopt

into an invariant check above loop.
Other passes may not be able to figure the fact that the new condition implies the old one, so such branch in %loop.next block:

loop.next:
  br i1 %cond.1, label %if.true, label %if.false

wouldn't get optimized.

This patch makes LoopPredication insert assumes of the replaced conditions either after a guard call or in the true block of widenable condition branch.

Diff Detail

Event Timeline

dmakogon created this revision.Oct 6 2022, 3:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 6 2022, 3:53 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
dmakogon requested review of this revision.Oct 6 2022, 3:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 6 2022, 3:53 AM

Do you have an exact motivation test where another pass fails to deduct facts from the new condition?

llvm/lib/Transforms/Scalar/LoopPredication.cpp
831

It's no longer dead.

846

Better use assert for it.

867

Same.

mkazantsev added inline comments.Oct 6 2022, 10:01 PM
llvm/lib/Transforms/Scalar/LoopPredication.cpp
831

Move to else block.

dmakogon updated this revision to Diff 465972.Oct 6 2022, 10:32 PM

Replaced llvm_unreachable with asserts

Do you have an exact motivation test where another pass fails to deduct facts from the new condition?

Yeah, it is test/Transforms/LoopPredication/assumes.ll

mkazantsev accepted this revision.Oct 6 2022, 11:54 PM

LG. I have a concern of potential code size growth, but on the other hand it can be easily switched off if it ever becomes a problem.

This revision is now accepted and ready to land.Oct 6 2022, 11:54 PM
dmakogon updated this revision to Diff 465977.Oct 7 2022, 12:03 AM

RecursivelyDeleteTriviallyDeadInstructions must be called anyway

dmakogon added inline comments.Oct 7 2022, 12:08 AM
llvm/lib/Transforms/Scalar/LoopPredication.cpp
831

Actually it is dead. The OldCond here would be something like

%explicit_guard_cond = and i1 %cond.1, %wc

and it is no longer used in a branch, however it makes use of widenable condition.
So there are two uses of it: the new and inserted by LoopPredication and this old one.
We require that widenable condition has exactly one use, so we have to remove it.
cond.1 is no longer dead, not this one

mkazantsev added inline comments.Oct 7 2022, 12:09 AM
llvm/lib/Transforms/Scalar/LoopPredication.cpp
831

Ah, yes. agreed. Thanks!