chandlerc added inline comments.
================
Comment at: clang/test/Misc/loop-opt-setup.c:12
+// Check br i1 to make sure that the loop is fully unrolled
// CHECK-NOT: br i1
----------------
This is dead now that you have different prefixes...
================
Comment at: clang/test/Misc/loop-opt-setup.c:32-33
+
+// Check br i1 to make sure the loop is gone, there will still be a label
branch for the infinite loop.
+// CHECK-NEWPM-NOT: br i1
+
----------------
But for which function?
I'd rework these to be more modern `CHECK-LABEL` and affirmative checks for the
unrolling.
================
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
----------------
echristo wrote:
> 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
> }
Then switch to generated `CHECK` lines?
The checks we have here will pass easily after incorrect edits that cause this
test to not actually check what it intends to. I'd either check something that
constructively shows unrolling or generate exact checks (if its small enough to
be genuinely stable).
================
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:
----------------
echristo wrote:
> 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.
Sure, but even those will make future edits to this really frustrating with
irrelevant diff, etc.
And this test case can still be minimized. As an example, the function is
currently attributed as *optnone*. =/
I think you can just run this through the metarenamer pass?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71687/new/
https://reviews.llvm.org/D71687
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
