[PATCH] D88395: [LoopReroll] Fix rerolling loop with extra instructions
Michael Kruse via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 10 10:50:52 PST 2021
Meinersbur added a comment.
Thanks for all the effort you put into this and sorry you had to wait so long for somebody to look at this.
I don't think the problem here is the `LoopIncs`. The unrerollable instructions can as well depend on `%iv.next`. You patch would still reroll the following:
define void @unrerollable_simple([2 x i32]* nocapture %a) {
entry:
br label %loop
loop:
; base instruction
%iv = phi i64 [ 0, %entry ], [ %iv.next, %loop ]
; unrerollable instructions
%iv.next = add nuw nsw i64 %iv, 1
%stptrx = getelementptr inbounds [2 x i32], [2 x i32]* %a, i64 %iv.next, i64 0
store i32 999, i32* %stptrx, align 4
; extra simple arithmetic operations, used by root instructions
%plus20 = add nuw nsw i64 %iv, 20
%plus10 = add nuw nsw i64 %iv, 10
; root instruction 0
%ldptr0 = getelementptr inbounds [2 x i32], [2 x i32]* %a, i64 %plus20, i64 0
%value0 = load i32, i32* %ldptr0, align 4
%stptr0 = getelementptr inbounds [2 x i32], [2 x i32]* %a, i64 %plus10, i64 0
store i32 %value0, i32* %stptr0, align 4
; root instruction 1
%ldptr1 = getelementptr inbounds [2 x i32], [2 x i32]* %a, i64 %plus20, i64 1
%value1 = load i32, i32* %ldptr1, align 4
%stptr1 = getelementptr inbounds [2 x i32], [2 x i32]* %a, i64 %plus10, i64 1
store i32 %value1, i32* %stptr1, align 4
; loop-increment and latch
;%iv.next = add nuw nsw i64 %iv, 1
%exitcond = icmp eq i64 %iv.next, 5
br i1 %exitcond, label %exit, label %loop
exit:
ret void
}
To me the problem looks like that the unrerollable store does not belong to any root set and is skipped during verification.
In D88395#2299624 <https://reviews.llvm.org/D88395#2299624>, @kawashima-fj wrote:
> `clang-format` reported a format error `DenseSet<Instruction*>` -> `DenseSet<Instruction *>` (space before `*`). However, other three places in the same function (and many places in other functions) have the same format. Should I change all places? only my change?, or keep all them unchanged?
Use the clang-format suggestion. To goal is to eventually converge to a completely clang-formatted codebase.
================
Comment at: llvm/lib/Transforms/Scalar/LoopRerollPass.cpp:1076
+ collectInLoopUserSet(IV, Exclude, PossibleRedSet, V);
+ for (auto *I : V) {
+ if (I != IV && !isSimpleArithmeticOp(I))
----------------
[style] LLVM's coding style does not make use of [[ https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable | Almost-Always-Auto ]].
================
Comment at: llvm/test/Transforms/LoopReroll/extra_instr.ll:12
+
+;CHECK-LABEL: @rerollable_simple
+;CHECK: loop:
----------------
[nit] There's usually a space before `CHECK`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88395/new/
https://reviews.llvm.org/D88395
More information about the llvm-commits
mailing list