[PATCH] D104148: [LoopUtils] Fix incorrect RT check bounds for loop-invariant mem accesses

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jul 10 06:47:46 PDT 2021


fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, thanks! It would be good to clean up `llvm/test/Transforms/LoopVectorize/pr50686.ll` a bit as well, to make maintaining it easier in the future.



================
Comment at: llvm/test/Transforms/LoopVectorize/pr50686.ll:94
 
 for.body.preheader:                               ; preds = %entry
   br label %for.body
----------------
Could you make the input IR a bit more compact by removing unnecessary BBs, potentially unnecessary instructions and adjust the names of the basic blocks so it is easier to spot the loops.

`%for.cond5.preheader` is misleadingly named for example, as it is *not* a pre-header.

This will make our live easier if the test needs updating in the future;


================
Comment at: llvm/test/Transforms/LoopVectorize/pr50686.ll:110
   store i32 2, i32* %arrayidx, align 4
   %i2 = load i32, i32* @k, align 4
   %inc = add nsw i32 %i2, 1
----------------
are all memory accesses needed here?


================
Comment at: llvm/test/Transforms/LoopVectorize/pr50686.ll:118
   %indvars.iv = phi i64 [ 0, %for.cond2.preheader ], [ %indvars.iv.next, %for.cond5.preheader ]
   %i3 = load i32, i32* %p2, align 4
   %sub = sub nsw i32 0, %i3
----------------
are all memory accesses here needed to reproduce the issue?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104148/new/

https://reviews.llvm.org/D104148



More information about the llvm-commits mailing list