fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.
LGTM, thanks!
================
Comment at: clang/test/Misc/loop-opt-setup.c:26
// Check br i1 to make sure the loop is gone, there will still be a label
branch for the infinite loop.
// CHECK-LABEL: Helper
----------------
fhahn wrote:
> This comment needs updating, there's no loop there now. Or better, add a
> run-line with a C standard version that does not have the forward progress
> guarantee, e.g. `-std=c99` and one with an explicit standard that has it and
> have different check lines for the 2 cases.
Can you also explain the C99 case in the comment?
================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:621
}
}
----------------
atmnpatel wrote:
> nikic wrote:
> > Unrelated, but why do these updates happen before the branch from preheader
> > to exit is added in IR? Shouldn't it be the other way around according to
> > the DTU contract?
> Isn't that branch added on line 602? My understanding was the changes on line
> 640 onwards are for removing, not introducing a branch.
> Shouldn't it be the other way around according to the DTU contract?
Yes I think so. Perhaps a missing case in the DTU validator. Probably good to
check/fix separately.
================
Comment at: llvm/test/Transforms/LoopDeletion/mustprogress.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
UTC_ARGS: --function-signature --check-attributes
+; RUN: opt < %s -loop-deletion -S | FileCheck %s
+
----------------
probably good to also add `-verify-dom-info`
================
Comment at: llvm/test/Transforms/LoopDeletion/mustprogress.ll:5
+;; Original C Code:
+;; void unknown_tripcount_mustprogress_attr_mustprogress_loopmd(int a, int b)
{
+;; for (; a < b;) ;
----------------
FWIW, I don't think the C code doesn't add much, but I don't have any strong
feelings about it. The IR is what is key and it's really small.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86844/new/
https://reviews.llvm.org/D86844
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
