[PATCH] D71687: Fix full loop unrolling initialization in new pass manager

[Eric Christopher via Phabricator via llvm-commits]

echristo marked 6 inline comments as done.
echristo added a comment.

OK, ready again :)



================
Comment at: llvm/test/Transforms/LoopUnroll/FullUnroll.ll:4-6
+; We don't end up deleting the loop, but we remove everything inside of it so checking for any
+; reasonable instruction from the original loop will work.
+; CHECK-NOT: br i1
----------------
chandlerc wrote:
> Make sure it is in the correct function at least, and maybe after the label for the loop header?
Got it. There's not a lot of function left at the end:

define void @walrus() local_unnamed_addr #0 {
entry:
  br label %for.cond.preheader

for.cond.preheader:                               ; preds = %for.cond.preheader, %entry
  br label %for.cond.preheader
}


================
Comment at: llvm/test/Transforms/LoopUnroll/FullUnroll.ll:12-17
+; Function Attrs: noinline optnone uwtable
+define dso_local void @_Z3Runv() #0 {
+entry:
+  call void @_Z6Helperv()
+  ret void
+}
----------------
chandlerc wrote:
> Are both functions needed?
I thought so, but apparently not :)


================
Comment at: llvm/test/Transforms/LoopUnroll/FullUnroll.ll:19-20
+
+; Function Attrs: noinline nounwind optnone uwtable
+define linkonce_odr dso_local void @_Z6Helperv() #1 comdat {
+entry:
----------------
chandlerc wrote:
> Nit, but minimize and clean this function up a touch?
> 
> At the least, removing all the target features seems valuable, and I'd give things stable names instead of numbered values.
Got it. Most things have stable names, only a few temporaries have numbered names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71687