[PATCH] D88395: [LoopReroll] Fix rerolling loop with extra instructions

[KAWASHIMA Takahiro via Phabricator via llvm-commits]

kawashima-fj created this revision.
kawashima-fj added reviewers: hfinkel, efriedma.
kawashima-fj added a project: LLVM.
Herald added subscribers: llvm-commits, hiraditya, kristof.beyls.
kawashima-fj requested review of this revision.

Fixes PR47627

This fix correctly suppresses rerolling a loop which has unrerollable
instructions which use the induction variable but are not used by root
instructions, reduction instructions, or a loop-increment instruction.

Sample IR for the explanation below:

  define void @foo([2 x i32]* nocapture %a) {
  entry:
    br label %loop
  
  loop:
    ; base instruction
    %indvar = phi i64 [ 0, %entry ], [ %indvar.next, %loop ]
  
    ; unrerollable instructions
    %stptrx = getelementptr inbounds [2 x i32], [2 x i32]* %a, i64 %indvar, i64 0
    store i32 999, i32* %stptrx, align 4
  
    ; extra simple arithmetic operations, used by root instructions
    %plus20 = add nuw nsw i64 %indvar, 20
    %plus10 = add nuw nsw i64 %indvar, 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
    %indvar.next = add nuw nsw i64 %indvar, 1
    %exitcond = icmp eq i64 %indvar.next, 5
    br i1 %exitcond, label %exit, label %loop
  
  exit:
    ret void
  }

Before this fix, `%indvar` was appended to the `LoopIncs` vector in
the `LoopReroll::DAGRootTracker::findRoots` function. I think this is
because we want to mark two instructions with
`extra simple arithmetic operations` comment above as rerollable
(`IL_All`) at the end of the
`LoopReroll::DAGRootTracker::collectUsedInstructions` function,
as well as three instructions with the `loop-increment and latch`
comment. However, this caused two instruction with the
`unrerollable instructions` comment above to be marked as rerollable
too.

After this fix, `%indvar` is not appended to the `LoopIncs` vector.
Instead, two instructions with `extra simple arithmetic operations`
comment are marked as rerollable using their own `collectInLoopUserSet`
call. The unrerollable `store` instruction is rejected by
`isSimpleArithmeticOp`. `isSimpleArithmeticOp` is used in the
`LoopReroll::DAGRootTracker::findRootsRecursive` function to accept
inctructions with the `extra simple arithmetic operations` comment.
In this logic, `%stptrx` is also marked as rerolloable but is harmless.

See https://bugs.llvm.org/show_bug.cgi?id=47627 for more information.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88395

Files:
  llvm/lib/Transforms/Scalar/LoopRerollPass.cpp
  llvm/test/Transforms/LoopReroll/extra_instr.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88395.294613.patch
Type: text/x-patch
Size: 21162 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200928/8adaebf8/attachment.bin>