fhahn created this revision.
fhahn added reviewers: jdoerfert, rjmccall, xbolva00, atmnpatel, aaron.ballman, 
rsmith.
fhahn requested review of this revision.
Herald added a project: clang.

Currently Clang does not add mustprogress to inifinite loops with a
known constant condition, matching C11 behavior. The forward progress
guarantee in C++11 and later should allow us to add mustprogress to any
loop (http://eel.is/c++draft/intro.progress#1).

This allows us to simplify the code dealing with adding mustprogress a
bit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96418

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-mustprogress.c
  clang/test/CodeGenCXX/attr-mustprogress.cpp

Index: clang/test/CodeGenCXX/attr-mustprogress.cpp
===================================================================
--- clang/test/CodeGenCXX/attr-mustprogress.cpp
+++ clang/test/CodeGenCXX/attr-mustprogress.cpp
@@ -10,27 +10,27 @@
 // CHECK: datalayout
 
 // CXX98-NOT: mustprogress
-// CXX11-NOT: mustprogress
+// CXX11:     mustprogress
 // CHECK-LABEL: @_Z2f0v(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    br label %for.cond
 // CHECK:       for.cond:
 // CXX98-NOT:    br {{.*}} llvm.loop
-// CXX11-NOT:    br {{.*}} llvm.loop
+// CXX11-NEXT:   br label %for.cond, !llvm.loop [[LOOP1:!.*]]
 void f0() {
   for (; ;) ;
 }
 
 // CXX98-NOT: mustprogress
-// CXX11-NOT: mustprogress
+// CXX11:     mustprogress
 // CHECK-LABEL: @_Z2f1v(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    br label %for.cond
 // CHECK:       for.cond:
 // CHECK-NEXT:    br i1 true, label %for.body, label %for.end
 // CHECK:       for.body:
-// CXX98-NOT:    br {{.*}}, !llvm.loop
-// CXX11-NOT:    br {{.*}}, !llvm.loop
+// CXX98-NOT:     br {{.*}}, !llvm.loop
+// CXX11-NEXT:    br label %for.cond, !llvm.loop [[LOOP2:!.*]]
 // CHECK:       for.end:
 // CHECK-NEXT:    ret void
 //
@@ -50,8 +50,8 @@
 // CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
 // CHECK-NEXT:    br i1 [[CMP]], label %for.body, label %for.end
 // CHECK:       for.body:
-// CXX98-NOT:    br {{.*}}, !llvm.loop
-// CXX11:        br label %for.cond, !llvm.loop [[LOOP1:!.*]]
+// CXX98-NOT:     br {{.*}}, !llvm.loop
+// CXX11-NEXT:    br label %for.cond, !llvm.loop [[LOOP3:!.*]]
 // CHECK:       for.end:
 // CHECK-NEXT:    ret void
 //
@@ -61,7 +61,7 @@
 }
 
 // CXX98-NOT: mustprogress
-// CXX11-NOT: mustprogress
+// CXX11:     mustprogress
 // CHECK-LABEL: @_Z1Fv(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    br label %for.cond
@@ -69,7 +69,7 @@
 // CHECK-NEXT:    br i1 true, label %for.body, label %for.end
 // CHECK:       for.body:
 // CXX98-NOT:     br {{.*}}, !llvm.loop
-// CXX11-NOT:     br {{.*}}, !llvm.loop
+// CXX11-NEXT:    br label %for.cond, !llvm.loop [[LOOP4:!.*]]
 // CHECK:       for.end:
 // CHECK-NEXT:    br label %for.cond1
 // CHECK:       for.cond1:
@@ -78,8 +78,8 @@
 // CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
 // CHECK-NEXT:    br i1 [[CMP]], label %for.body2, label %for.end3
 // CHECK:       for.body2:
-// CXX98-NOT:    br {{.*}}, !llvm.loop
-// CXX11:        br label %for.cond1, !llvm.loop [[LOOP2:!.*]]
+// CXX98-NOT:     br {{.*}}, !llvm.loop
+// CXX11-NEXT:    br label %for.cond1, !llvm.loop [[LOOP5:!.*]]
 // CHECK:       for.end3:
 // CHECK-NEXT:    ret void
 //
@@ -102,14 +102,14 @@
 // CHECK-NEXT:    br i1 [[CMP]], label %for.body, label %for.end
 // CHECK:       for.body:
 // CXX98_NOT:     br {{.*}} !llvm.loop
-// CXX11-NEXT:    br label %for.cond, !llvm.loop
+// CXX11-NEXT:    br label %for.cond, !llvm.loop [[LOOP6:!.*]]
 // CHECK:       for.end:
 // CHECK-NEXT:    br label %for.cond1
 // CHECK:       for.cond1:
 // CHECK-NEXT:    br i1 true, label %for.body2, label %for.end3
 // CHECK:       for.body2:
 // CXX98-NOT:     br {{.*}}, !llvm.loop
-// CXX11-NOT:     br {{.*}}, !llvm.loop
+// CXX11-NEXT:    br label %for.cond1, !llvm.loop [[LOOP7:!.*]]
 // CHECK:       for.end3:
 // CHECK-NEXT:    ret void
 //
@@ -121,13 +121,13 @@
 }
 
 // CXX98-NOT: mustprogress
-// CXX11-NOT: mustprogress
+// CXX11:     mustprogress
 // CHECK-LABEL: @_Z2w1v(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    br label %while.body
 // CHECK:       while.body:
 // CXX98-NOT:     br {{.*}}, !llvm.loop
-// CXX11-NOT:     br {{.*}}, !llvm.loop
+// CXX11-NEXT:    br label %while.body, !llvm.loop [[LOOP8:!.*]]
 //
 void w1() {
   while (1)
@@ -146,7 +146,7 @@
 // CHECK-NEXT:    br i1 [[CMP]], label %while.body, label %while.end
 // CHECK:       while.body:
 // CXX98-NOT:     br {{.*}}, !llvm.loop
-// CXX11-NEXT:    br label %while.cond, !llvm.loop [[LOOP3:!.*]]
+// CXX11-NEXT:    br label %while.cond, !llvm.loop [[LOOP9:!.*]]
 // CHECK:       while.end:
 // CHECK-NEXT:    ret void
 //
@@ -156,7 +156,7 @@
 }
 
 // CXX98-NOT: mustprogress
-// CXX11-NOT: mustprogress
+// CXX11:     mustprogress
 // CHECK-LABEL: @_Z1Wv(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    br label %while.cond
@@ -167,12 +167,12 @@
 // CHECK-NEXT:    br i1 [[CMP]], label %while.body, label %while.end
 // CHECK:       while.body:
 // CXX98-NOT:     br {{.*}}, !llvm.loop
-// CXX11-NEXT:    br label %while.cond, !llvm.loop [[LOOP4:!.*]]
+// CXX11-NEXT:    br label %while.cond, !llvm.loop [[LOOP10:!.*]]
 // CHECK:       while.end:
 // CHECK-NEXT:    br label %while.body2
 // CHECK:       while.body2:
 // CXX98-NOT:    br {{.*}}, !llvm.loop
-// CXX11-NOT:    br {{.*}}, !llvm.loop
+// CXX11-NEXT:   br label %while.body2, !llvm.loop [[LOOP11:!.*]]
 //
 void W() {
   while (a == b)
@@ -182,13 +182,13 @@
 }
 
 // CXX98-NOT: mustprogress
-// CXX11-NOT: mustprogress
+// CXX11:     mustprogress
 // CHECK-LABEL: @_Z2W2v(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    br label %while.body
 // CHECK:       while.body:
 // CXX98-NOT:     br {{.*}}, !llvm.loop
-// CXX11-NOT:     br {{.*}}, !llvm.loop
+// CXX11-NEXT:    br label %while.body, !llvm.loop [[LOOP12:!.*]]
 //
 void W2() {
   while (1)
@@ -198,7 +198,7 @@
 }
 
 // CXX98-NOT: mustprogress
-// CXX11-NOT: mustprogress
+// CXX11:     mustprogress
 // CHECK-LABEL: @_Z2d1v(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    br label %do.body
@@ -206,7 +206,7 @@
 // CHECK-NEXT:    br label %do.cond
 // CHECK:       do.cond:
 // CXX98-NOT:     br {{.*}}, !llvm.loop
-// CXX11-NOT:     br {{.*}}, !llvm.loop
+// CXX11-NEXT:    br i1 true, label %do.body, label %do.end, !llvm.loop [[LOOP13:!.*]]
 // CHECK:       do.end:
 // CHECK-NEXT:    ret void
 //
@@ -228,7 +228,7 @@
 // CHECK-NEXT:    [[TMP1:%.*]] = load i32, i32* @b, align 4
 // CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
 // CXX98-NOT:     br {{.*}}, !llvm.loop
-// CXX11-NEXT:    br i1 [[CMP]], label %do.body, label %do.end, !llvm.loop [[LOOP5:!.*]]
+// CXX11-NEXT:    br i1 [[CMP]], label %do.body, label %do.end, !llvm.loop [[LOOP14:!.*]]
 // CHECK:       do.end:
 // CHECK-NEXT:    ret void
 //
@@ -239,7 +239,7 @@
 }
 
 // CXX98-NOT: mustprogress
-// CXX11-NOT: mustprogress
+// CXX11:     mustprogress
 // CHECK-LABEL: @_Z1Dv(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    br label %do.body
@@ -247,7 +247,7 @@
 // CHECK-NEXT:    br label %do.cond
 // CHECK:       do.cond:
 // CXX98-NOT:     br {{.*}}, !llvm.loop
-// CXX11-NOT:     br {{.*}}, !llvm.loop
+// CXX11-NEXT:     br i1 true, label %do.body, label %do.end, !llvm.loop [[LOOP15:!.*]]
 // CHECK:       do.end:
 // CHECK-NEXT:    br label %do.body1
 // CHECK:       do.body1:
@@ -257,7 +257,7 @@
 // CHECK-NEXT:    [[TMP1:%.*]] = load i32, i32* @b, align 4
 // CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
 // CXX98-NOT:     br {{.*}}, !llvm.loop
-// CXX11-NEXT:    br i1 [[CMP]], label %do.body1, label %do.end3, !llvm.loop [[LOOP6:!.*]]
+// CXX11-NEXT:    br i1 [[CMP]], label %do.body1, label %do.end3, !llvm.loop [[LOOP16:!.*]]
 // CHECK:       do.end3:
 // CHECK-NEXT:    ret void
 //
@@ -271,7 +271,7 @@
 }
 
 // CXX98-NOT: mustprogress
-// CXX11-NOT: mustprogress
+// CXX11:     mustprogress
 // CHECK-LABEL: @_Z2D2v(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    br label %do.body
@@ -282,14 +282,14 @@
 // CHECK-NEXT:    [[TMP1:%.*]] = load i32, i32* @b, align 4
 // CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
 // CXX98-NOT:     br {{.*}}, !llvm.loop
-// CXX11-NEXT:    br i1 [[CMP]], label %do.body, label %do.end, !llvm.loop [[LOOP7:!.*]]
+// CXX11-NEXT:    br i1 [[CMP]], label %do.body, label %do.end, !llvm.loop [[LOOP17:!.*]]
 // CHECK:       do.end:
 // CHECK-NEXT:    br label %do.body1
 // CHECK:       do.body1:
 // CHECK-NEXT:    br label %do.cond2
 // CHECK:       do.cond2:
 // CXX98-NOT:     br {{.*}}, !llvm.loop
-// CXX11-NOT:     br {{.*}}, !llvm.loop
+// CXX11-NEXT:    br i1 true, label %do.body1, label %do.end3, !llvm.loop [[LOOP18:!.*]]
 // CHECK:       do.end3:
 // CHECK-NEXT:    ret void
 //
@@ -302,7 +302,6 @@
   while (1);
 }
 
-
 // CXX11: [[LOOP1]] = distinct !{[[LOOP1]], [[MP:!.*]]}
 // CXX11: [[MP]] = !{!"llvm.loop.mustprogress"}
 // CXX11: [[LOOP2]] = distinct !{[[LOOP2]], [[MP]]}
@@ -311,3 +310,14 @@
 // CXX11: [[LOOP5]] = distinct !{[[LOOP5]], [[MP]]}
 // CXX11: [[LOOP6]] = distinct !{[[LOOP6]], [[MP]]}
 // CXX11: [[LOOP7]] = distinct !{[[LOOP7]], [[MP]]}
+// CXX11: [[LOOP8]] = distinct !{[[LOOP8]], [[MP]]}
+// CXX11: [[LOOP9]] = distinct !{[[LOOP9]], [[MP]]}
+// CXX11: [[LOOP10]] = distinct !{[[LOOP10]], [[MP]]}
+// CXX11: [[LOOP11]] = distinct !{[[LOOP11]], [[MP]]}
+// CXX11: [[LOOP12]] = distinct !{[[LOOP12]], [[MP]]}
+// CXX11: [[LOOP13]] = distinct !{[[LOOP13]], [[MP]]}
+// CXX11: [[LOOP14]] = distinct !{[[LOOP14]], [[MP]]}
+// CXX11: [[LOOP15]] = distinct !{[[LOOP15]], [[MP]]}
+// CXX11: [[LOOP16]] = distinct !{[[LOOP16]], [[MP]]}
+// CXX11: [[LOOP17]] = distinct !{[[LOOP17]], [[MP]]}
+// CXX11: [[LOOP18]] = distinct !{[[LOOP18]], [[MP]]}
Index: clang/test/CodeGen/attr-mustprogress.c
===================================================================
--- clang/test/CodeGen/attr-mustprogress.c
+++ clang/test/CodeGen/attr-mustprogress.c
@@ -104,7 +104,7 @@
 // CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
 // CHECK-NEXT:    br i1 [[CMP]], label %while.body, label %while.end
 // CHECK:       while.body:
-// C11:           br label %while.cond, [[LOOP5:!llvm.loop !.*]]
+// C11:           br label %while.cond, !llvm.loop [[LOOP3:!.*]]
 // C99-NOT:       br {{.*}}, !llvm.loop
 // CHECK:       while.end:
 // CHECK-NEXT:    ret void
@@ -125,7 +125,9 @@
 // CHECK-NEXT:    br i1 [[CMP]], label %while.body, label %while.end
 // CHECK:       while.body:
 // C99-NOT:       br {{.*}} !llvm.loop
-// C11:           br label %while.body, !llvm.loop [[LOOP3:!.*]]
+// C11-NEXT:      br label %while.cond, !llvm.loop [[LOOP4:!.*]]
+// CHECK:       while.body2:
+// CHECK-NOT:     br {{.*}} !llvm.loop
 //
 void W() {
   while (a == b) {
@@ -161,7 +163,7 @@
 // CHECK-NEXT:    [[TMP1:%.*]] = load i32, i32* @b, align 4
 // CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
 // C99-NOT:       br {{.*}}, !llvm.loop
-// C11:           br i1 [[CMP]], label %do.body, label %do.end, !llvm.loop [[LOOP4:!.*]]
+// C11:           br i1 [[CMP]], label %do.body, label %do.end, !llvm.loop [[LOOP5:!.*]]
 // CHECK:       do.end:
 // CHECK-NEXT:    ret void
 //
@@ -187,7 +189,7 @@
 // CHECK-NEXT:    [[TMP1:%.*]] = load i32, i32* @b, align 4
 // CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
 // C99-NOT:       br {{.*}}, !llvm.loop
-// C11:           br i1 [[CMP]], label %do.body1, label %do.end3, !llvm.loop [[LOOP5:!.*]]
+// C11:           br i1 [[CMP]], label %do.body1, label %do.end3, !llvm.loop [[LOOP6:!.*]]
 // CHECK:       do.end3:
 // CHECK-NEXT:    ret void
 //
@@ -204,3 +206,4 @@
 // C11: [[LOOP3]] = distinct !{[[LOOP3]], [[MP]]}
 // C11: [[LOOP4]] = distinct !{[[LOOP4]], [[MP]]}
 // C11: [[LOOP5]] = distinct !{[[LOOP5]], [[MP]]}
+// C11: [[LOOP6]] = distinct !{[[LOOP6]], [[MP]]}
Index: clang/lib/CodeGen/CodeGenFunction.h
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -505,23 +505,23 @@
   /// True if the current function should be marked mustprogress.
   bool FnIsMustProgress = false;
 
-  /// True if the C++ Standard Requires Progress.
-  bool CPlusPlusWithProgress() {
+  /// Returns true if the function must make progress.
+  bool functionMustProgress() {
     return getLangOpts().CPlusPlus11 || getLangOpts().CPlusPlus14 ||
            getLangOpts().CPlusPlus17 || getLangOpts().CPlusPlus20;
   }
 
-  /// True if the C Standard Requires Progress.
-  bool CWithProgress() {
+  /// Returns true if either the containing function must make progress.
+  /// Otherwise return false if the loop condition is a known constant and true
+  /// if the language standard is C11+.
+  bool loopMustProgress(bool HasConstantCond) {
+    if (functionMustProgress())
+      return true;
+    if (HasConstantCond)
+      return false;
     return getLangOpts().C11 || getLangOpts().C17 || getLangOpts().C2x;
   }
 
-  /// True if the language standard requires progress in functions or
-  /// in infinite loops with non-constant conditionals.
-  bool LanguageRequiresProgress() {
-    return CWithProgress() || CPlusPlusWithProgress();
-  }
-
   const CodeGen::CGBlockInfo *BlockInfo = nullptr;
   llvm::Value *BlockPointer = nullptr;
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1177,9 +1177,6 @@
 
 void CodeGenFunction::EmitFunctionBody(const Stmt *Body) {
   incrementProfileCounter(Body);
-  if (CPlusPlusWithProgress())
-    FnIsMustProgress = true;
-
   if (const CompoundStmt *S = dyn_cast<CompoundStmt>(Body))
     EmitCompoundStmtWithoutScope(*S);
   else
@@ -1187,7 +1184,7 @@
 
   // This is checked after emitting the function body so we know if there
   // are any permitted infinite loops.
-  if (FnIsMustProgress)
+  if (functionMustProgress())
     CurFn->addFnAttr(llvm::Attribute::MustProgress);
 }
 
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -791,20 +791,14 @@
 
   // while(1) is common, avoid extra exit blocks.  Be sure
   // to correctly handle break/continue though.
-  bool EmitBoolCondBranch = true;
-  bool LoopMustProgress = false;
-  if (llvm::ConstantInt *C = dyn_cast<llvm::ConstantInt>(BoolCondVal)) {
-    if (C->isOne()) {
-      EmitBoolCondBranch = false;
-      FnIsMustProgress = false;
-    }
-  } else if (LanguageRequiresProgress())
-    LoopMustProgress = true;
-
+  llvm::ConstantInt *C = dyn_cast<llvm::ConstantInt>(BoolCondVal);
+  bool EmitBoolCondBranch = !C || !C->isOne();
+  bool CondIsConst = C;
   const SourceRange &R = S.getSourceRange();
   LoopStack.push(LoopHeader.getBlock(), CGM.getContext(), CGM.getCodeGenOpts(),
                  WhileAttrs, SourceLocToDebugLoc(R.getBegin()),
-                 SourceLocToDebugLoc(R.getEnd()), LoopMustProgress);
+                 SourceLocToDebugLoc(R.getEnd()),
+                 loopMustProgress(CondIsConst));
 
   // As long as the condition is true, go to the loop body.
   llvm::BasicBlock *LoopBody = createBasicBlock("while.body");
@@ -892,20 +886,15 @@
 
   // "do {} while (0)" is common in macros, avoid extra blocks.  Be sure
   // to correctly handle break/continue though.
-  bool EmitBoolCondBranch = true;
-  bool LoopMustProgress = false;
-  if (llvm::ConstantInt *C = dyn_cast<llvm::ConstantInt>(BoolCondVal)) {
-    if (C->isZero())
-      EmitBoolCondBranch = false;
-    else if (C->isOne())
-      FnIsMustProgress = false;
-  } else if (LanguageRequiresProgress())
-    LoopMustProgress = true;
+  llvm::ConstantInt *C = dyn_cast<llvm::ConstantInt>(BoolCondVal);
+  bool CondIsConst = C;
+  bool EmitBoolCondBranch = !C || !C->isZero();
 
   const SourceRange &R = S.getSourceRange();
   LoopStack.push(LoopBody, CGM.getContext(), CGM.getCodeGenOpts(), DoAttrs,
                  SourceLocToDebugLoc(R.getBegin()),
-                 SourceLocToDebugLoc(R.getEnd()), LoopMustProgress);
+                 SourceLocToDebugLoc(R.getEnd()),
+                 loopMustProgress(CondIsConst));
 
   // As long as the condition is true, iterate the loop.
   if (EmitBoolCondBranch) {
@@ -943,20 +932,15 @@
   llvm::BasicBlock *CondBlock = Continue.getBlock();
   EmitBlock(CondBlock);
 
-  bool LoopMustProgress = false;
   Expr::EvalResult Result;
-  if (LanguageRequiresProgress()) {
-    if (!S.getCond()) {
-      FnIsMustProgress = false;
-    } else if (!S.getCond()->EvaluateAsInt(Result, getContext())) {
-      LoopMustProgress = true;
-    }
-  }
+  bool CondIsConst =
+      !S.getCond() || S.getCond()->EvaluateAsInt(Result, getContext());
 
   const SourceRange &R = S.getSourceRange();
   LoopStack.push(CondBlock, CGM.getContext(), CGM.getCodeGenOpts(), ForAttrs,
                  SourceLocToDebugLoc(R.getBegin()),
-                 SourceLocToDebugLoc(R.getEnd()), LoopMustProgress);
+                 SourceLocToDebugLoc(R.getEnd()),
+                 loopMustProgress(CondIsConst));
 
   // If the for loop doesn't have an increment we can just use the
   // condition as the continue block.  Otherwise we'll need to create
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to