llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: cor3ntin (cor3ntin) <details> <summary>Changes</summary> https://wg21.link/P2809R3 This is applied as a DR to C++11 (C++98 did not guarantee forward progress and is left untouched) As an extension (and to preserve existing behavior in C), we consider all controlling expression that can be constant folded in the front end, not just standard constant expressions. --- Full diff: https://github.com/llvm/llvm-project/pull/90066.diff 6 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+3-1) - (modified) clang/lib/CodeGen/CGStmt.cpp (+71-10) - (modified) clang/lib/CodeGen/CodeGenFunction.cpp (+1) - (modified) clang/lib/CodeGen/CodeGenFunction.h (+1-22) - (modified) clang/test/CodeGenCXX/attr-mustprogress.cpp (+30-40) - (modified) clang/www/cxx_status.html (+1-1) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 64526ed6d06f55..f5906c2dd4eb52 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -122,7 +122,7 @@ C++23 Feature Support materialize temporary object which is a prvalue in discarded-value expression. - Implemented `P1774R8: Portable assumptions <https://wg21.link/P1774R8>`_. -- Implemented `P2448R2: Relaxing some constexpr restrictions <https://wg21.link/P2448R2>`_. +- Implemented `P2809R3: Trivial infinite loops are not Undefined Behavior <https://wg21.link/P2809R3>`_. C++2c Feature Support ^^^^^^^^^^^^^^^^^^^^^ @@ -131,6 +131,8 @@ C++2c Feature Support - Implemented `P2573R2: = delete("should have a reason"); <https://wg21.link/P2573R2>`_ +- Implemented `P2573R2: = delete("should have a reason"); <https://wg21.link/P2573R2>`_ + Resolutions to C++ Defect Reports ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index 576fe2f7a2d46f..7a0ad8a73b9fce 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -908,6 +908,73 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) { incrementProfileCounter(&S); } +bool CodeGenFunction::checkIfLoopMustProgress(const Expr *ControllingExpression, + bool IsTrivialCXXLoop) { + if (CGM.getCodeGenOpts().getFiniteLoops() == + CodeGenOptions::FiniteLoopsKind::Always) + return true; + if (CGM.getCodeGenOpts().getFiniteLoops() == + CodeGenOptions::FiniteLoopsKind::Never) + return false; + + // Now apply rules for plain C (see 6.8.5.6 in C11). + // Loops with constant conditions do not have to make progress in any C + // version. + // As an extension, we consisider loops whose constant expression + // can be constant-folded. + Expr::EvalResult Result; + bool CondIsConstInt = + !ControllingExpression || + (ControllingExpression->EvaluateAsInt(Result, getContext()) && + Result.Val.isInt()); + bool IsTrue = CondIsConstInt && + (!ControllingExpression || Result.Val.getInt().getBoolValue()); + + if (getLangOpts().C99 && CondIsConstInt) + return false; + + // Loops with non-constant conditions must make progress in C11 and later. + if (getLangOpts().C11) + return true; + + // [C++26][intro.progress] (DR) + // The implementation may assume that any thread will eventually do one of the + // following: + // [...] + // - continue execution of a trivial infinite loop ([stmt.iter.general]). + if (getLangOpts().CPlusPlus11) { + if (IsTrivialCXXLoop && IsTrue) { + CurFn->removeFnAttr(llvm::Attribute::MustProgress); + return false; + } + return true; + } + + return false; +} + +// [C++26][stmt.iter.general] (DR) +// A trivially empty iteration statement is an iteration statement matching one +// of the following forms: +// - while ( expression ) ; +// - while ( expression ) { } +// - do ; while ( expression ) ; +// - do { } while ( expression ) ; +// - for ( init-statement expression(opt); ) ; +// - for ( init-statement expression(opt); ) { } +template <typename LoopStmt> static bool hasEmptyLoopBody(const LoopStmt &S) { + if constexpr (std::is_same_v<LoopStmt, ForStmt>) { + if (S.getInc()) + return false; + } + const Stmt *Body = S.getBody(); + if (!Body || isa<NullStmt>(Body)) + return true; + if (const CompoundStmt *Compound = dyn_cast<CompoundStmt>(Body)) + return Compound->body_empty(); + return false; +} + void CodeGenFunction::EmitWhileStmt(const WhileStmt &S, ArrayRef<const Attr *> WhileAttrs) { // Emit the header for the loop, which will also become @@ -942,13 +1009,12 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt &S, // while(1) is common, avoid extra exit blocks. Be sure // to correctly handle break/continue though. llvm::ConstantInt *C = dyn_cast<llvm::ConstantInt>(BoolCondVal); - bool CondIsConstInt = C != nullptr; - bool EmitBoolCondBranch = !CondIsConstInt || !C->isOne(); + bool EmitBoolCondBranch = !C || !C->isOne(); const SourceRange &R = S.getSourceRange(); LoopStack.push(LoopHeader.getBlock(), CGM.getContext(), CGM.getCodeGenOpts(), WhileAttrs, SourceLocToDebugLoc(R.getBegin()), SourceLocToDebugLoc(R.getEnd()), - checkIfLoopMustProgress(CondIsConstInt)); + checkIfLoopMustProgress(S.getCond(), hasEmptyLoopBody(S))); // When single byte coverage mode is enabled, add a counter to loop condition. if (llvm::EnableSingleByteCoverage) @@ -1059,14 +1125,13 @@ void CodeGenFunction::EmitDoStmt(const DoStmt &S, // "do {} while (0)" is common in macros, avoid extra blocks. Be sure // to correctly handle break/continue though. llvm::ConstantInt *C = dyn_cast<llvm::ConstantInt>(BoolCondVal); - bool CondIsConstInt = 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()), - checkIfLoopMustProgress(CondIsConstInt)); + checkIfLoopMustProgress(S.getCond(), hasEmptyLoopBody(S))); // As long as the condition is true, iterate the loop. if (EmitBoolCondBranch) { @@ -1109,15 +1174,11 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S, llvm::BasicBlock *CondBlock = CondDest.getBlock(); EmitBlock(CondBlock); - Expr::EvalResult Result; - bool CondIsConstInt = - !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()), - checkIfLoopMustProgress(CondIsConstInt)); + checkIfLoopMustProgress(S.getCond(), hasEmptyLoopBody(S))); // Create a cleanup scope for the condition variable cleanups. LexicalScope ConditionScope(*this, S.getSourceRange()); diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 86a6ddd80cc114..95c0784ad9760c 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1465,6 +1465,7 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn, // Ensure that the function adheres to the forward progress guarantee, which // is required by certain optimizations. + // The attribute will be removed if the body contains a trivial empty loop. if (checkIfFunctionMustProgress()) CurFn->addFnAttr(llvm::Attribute::MustProgress); diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index ff1873325d409f..f0e8e47bc0e420 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -635,28 +635,7 @@ class CodeGenFunction : public CodeGenTypeCache { /// Returns true if a loop must make progress, which means the mustprogress /// attribute can be added. \p HasConstantCond indicates whether the branch /// condition is a known constant. - bool checkIfLoopMustProgress(bool HasConstantCond) { - if (CGM.getCodeGenOpts().getFiniteLoops() == - CodeGenOptions::FiniteLoopsKind::Always) - return true; - if (CGM.getCodeGenOpts().getFiniteLoops() == - CodeGenOptions::FiniteLoopsKind::Never) - return false; - - // If the containing function must make progress, loops also must make - // progress (as in C++11 and later). - if (checkIfFunctionMustProgress()) - return true; - - // Now apply rules for plain C (see 6.8.5.6 in C11). - // Loops with constant conditions do not have to make progress in any C - // version. - if (HasConstantCond) - return false; - - // Loops with non-constant conditions must make progress in C11 and later. - return getLangOpts().C11; - } + bool checkIfLoopMustProgress(const Expr *, bool IsTrivialCXXLoop); const CodeGen::CGBlockInfo *BlockInfo = nullptr; llvm::Value *BlockPointer = nullptr; diff --git a/clang/test/CodeGenCXX/attr-mustprogress.cpp b/clang/test/CodeGenCXX/attr-mustprogress.cpp index 843f5460426ccf..5a24e9426d1308 100644 --- a/clang/test/CodeGenCXX/attr-mustprogress.cpp +++ b/clang/test/CodeGenCXX/attr-mustprogress.cpp @@ -24,21 +24,21 @@ int b = 0; // CHECK: datalayout // CXX98-NOT: mustprogress -// CXX11: mustprogress +// CXX11-NOT: mustprogress // FINITE-NOT: mustprogress // CHECK-LABEL: @_Z2f0v( // CHECK-NEXT: entry: // CHECK-NEXT: br label %for.cond // CHECK: for.cond: // CXX98-NOT: br {{.*}} llvm.loop -// CXX11-NEXT: br label %for.cond, !llvm.loop [[LOOP1:!.*]] +// CXX11-NOT: br {{.*}} llvm.loop // FINITE-NEXT: br label %for.cond, !llvm.loop [[LOOP1:!.*]] void f0() { for (; ;) ; } // CXX98-NOT: mustprogress -// CXX11: mustprogress +// CXX11-NOT: mustprogress // FINITE-NOT: mustprogress // CHECK-LABEL: @_Z2f1v( // CHECK-NEXT: entry: @@ -46,8 +46,8 @@ void f0() { // CHECK: for.cond: // CHECK-NEXT: br i1 true, label %for.body, label %for.end // CHECK: for.body: -// CXX98-NOT: br {{.*}}, !llvm.loop -// CXX11-NEXT: br label %for.cond, !llvm.loop [[LOOP2:!.*]] +// CXX98-NOT: br {{.*}}, !llvm.loop +// CXX11-NOT: br {{.*}} llvm.loop // FINITE-NEXT: br label %for.cond, !llvm.loop [[LOOP2:!.*]] // CHECK: for.end: // CHECK-NEXT: ret void @@ -81,7 +81,7 @@ void f2() { } // CXX98-NOT: mustprogress -// CXX11: mustprogress +// CXX11-NOT: mustprogress // FINITE-NOT: mustprogress // CHECK-LABEL: @_Z1Fv( // CHECK-NEXT: entry: @@ -90,7 +90,7 @@ void f2() { // CHECK-NEXT: br i1 true, label %for.body, label %for.end // CHECK: for.body: // CXX98-NOT: br {{.*}}, !llvm.loop -// CXX11-NEXT: br label %for.cond, !llvm.loop [[LOOP4:!.*]] +// CXX11-NOT: br {{.*}}, !llvm.loop // FINITE-NEXT: br label %for.cond, !llvm.loop [[LOOP4:!.*]] // CHECK: for.end: // CHECK-NEXT: br label %for.cond1 @@ -114,7 +114,7 @@ void F() { } // CXX98-NOT: mustprogress -// CXX11: mustprogress +// CXX11-NOT: mustprogress // FINITE-NOT: mustprogress // CHECK-LABEL: @_Z2F2v( // CHECK-NEXT: entry: @@ -134,7 +134,7 @@ void F() { // CHECK-NEXT: br i1 true, label %for.body2, label %for.end3 // CHECK: for.body2: // CXX98-NOT: br {{.*}}, !llvm.loop -// CXX11-NEXT: br label %for.cond1, !llvm.loop [[LOOP7:!.*]] +// CXX11-NOT: br {{.*}}, !llvm.loop // FINITE-NEXT: br label %for.cond1, !llvm.loop [[LOOP7:!.*]] // CHECK: for.end3: // CHECK-NEXT: ret void @@ -147,14 +147,14 @@ void F2() { } // CXX98-NOT: mustprogress -// CXX11: mustprogress +// CXX11-NOT: mustprogress // FINITE-NOT: mustprogress // CHECK-LABEL: @_Z2w1v( // CHECK-NEXT: entry: // CHECK-NEXT: br label %while.body // CHECK: while.body: // CXX98-NOT: br {{.*}}, !llvm.loop -// CXX11-NEXT: br label %while.body, !llvm.loop [[LOOP8:!.*]] +// CXX11-NOT: br {{.*}}, !llvm.loop // FINITE-NEXT: br label %while.body, !llvm.loop [[LOOP8:!.*]] // void w1() { @@ -186,7 +186,7 @@ void w2() { } // CXX98-NOT: mustprogress -// CXX11: mustprogress +// CXX11-NOT: mustprogress // FINITE-NOT: mustprogress // CHECK-LABEL: @_Z1Wv( // CHECK-NEXT: entry: @@ -204,7 +204,7 @@ void w2() { // CHECK-NEXT: br label %while.body2 // CHECK: while.body2: // CXX98-NOT: br {{.*}}, !llvm.loop -// CXX11-NEXT: br label %while.body2, !llvm.loop [[LOOP11:!.*]] +// CXX11-NOT: br {{.*}}, !llvm.loop // FINITE-NEXT: br label %while.body2, !llvm.loop [[LOOP11:!.*]] // void W() { @@ -215,14 +215,14 @@ void W() { } // CXX98-NOT: mustprogress -// CXX11: mustprogress +// CXX11-NOT: mustprogress // FINITE-NOT: mustprogress // CHECK-LABEL: @_Z2W2v( // CHECK-NEXT: entry: // CHECK-NEXT: br label %while.body // CHECK: while.body: // CXX98-NOT: br {{.*}}, !llvm.loop -// CXX11-NEXT: br label %while.body, !llvm.loop [[LOOP12:!.*]] +// CXX11-NOT: br label %while.body, !llvm.loop [[LOOP12:!.*]] // FINITE-NEXT: br label %while.body, !llvm.loop [[LOOP12:!.*]] // void W2() { @@ -233,7 +233,7 @@ void W2() { } // CXX98-NOT: mustprogress -// CXX11: mustprogress +// CXX11-NOT: mustprogress // FINITE-NOT: mustprogress // CHECK-LABEL: @_Z2d1v( // CHECK-NEXT: entry: @@ -242,7 +242,7 @@ void W2() { // CHECK-NEXT: br label %do.cond // CHECK: do.cond: // CXX98-NOT: br {{.*}}, !llvm.loop -// CXX11-NEXT: br i1 true, label %do.body, label %do.end, !llvm.loop [[LOOP13:!.*]] +// CXX11-NOT: br {{.*}}, !llvm.loop // FINITE-NEXT: br i1 true, label %do.body, label %do.end, !llvm.loop [[LOOP13:!.*]] // CHECK: do.end: // CHECK-NEXT: ret void @@ -278,7 +278,7 @@ void d2() { } // CXX98-NOT: mustprogress -// CXX11: mustprogress +// CXX11-NOT: mustprogress // FINITE-NOT: mustprogress // CHECK-LABEL: @_Z1Dv( // CHECK-NEXT: entry: @@ -287,7 +287,7 @@ void d2() { // CHECK-NEXT: br label %do.cond // CHECK: do.cond: // CXX98-NOT: br {{.*}}, !llvm.loop -// CXX11-NEXT: br i1 true, label %do.body, label %do.end, !llvm.loop [[LOOP15:!.*]] +// CXX11-NOT: br {{.*}}, !llvm.loop // FINITE-NEXT: br i1 true, label %do.body, label %do.end, !llvm.loop [[LOOP15:!.*]] // CHECK: do.end: // CHECK-NEXT: br label %do.body1 @@ -312,8 +312,8 @@ void D() { while (a == b); } -// CXX98-NOT : mustprogress -// CXX11: mustprogress +// CXX98-NOT: mustprogress +// CXX11-NOT: mustprogress // FINITE-NOT: mustprogress // CHECK-LABEL: @_Z2D2v( // CHECK-NEXT: entry: @@ -333,7 +333,7 @@ void D() { // CHECK-NEXT: br label %do.cond2 // CHECK: do.cond2: // CXX98-NOT: br {{.*}}, !llvm.loop -// CXX11-NEXT: br i1 true, label %do.body1, label %do.end3, !llvm.loop [[LOOP18:!.*]] +// CXX11-NOT: br {{.*}}, !llvm.loop // FINITE-NEXT: br i1 true, label %do.body1, label %do.end3, !llvm.loop [[LOOP18:!.*]] // CHECK: do.end3: // CHECK-NEXT: ret void @@ -347,22 +347,12 @@ void D2() { while (1); } -// CXX11: [[LOOP1]] = distinct !{[[LOOP1]], [[MP:!.*]]} +// CXX11: [[LOOP1:.*]] = distinct !{[[LOOP1]], [[MP:.*]]} // CXX11: [[MP]] = !{!"llvm.loop.mustprogress"} -// CXX11: [[LOOP2]] = distinct !{[[LOOP2]], [[MP]]} -// CXX11: [[LOOP3]] = distinct !{[[LOOP3]], [[MP]]} -// CXX11: [[LOOP4]] = distinct !{[[LOOP4]], [[MP]]} -// 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]]} +// CXX11: [[LOOP2:.*]] = distinct !{[[LOOP2]], [[MP]]} +// CXX11: [[LOOP3:.*]] = distinct !{[[LOOP3]], [[MP]]} +// CXX11: [[LOOP4:.*]] = distinct !{[[LOOP4]], [[MP]]} +// CXX11: [[LOOP5:.*]] = distinct !{[[LOOP5]], [[MP]]} +// CXX11: [[LOOP6:.*]] = distinct !{[[LOOP6]], [[MP]]} +// CXX11: [[LOOP7:.*]] = distinct !{[[LOOP7]], [[MP]]} +// CXX11: [[LOOP8:.*]] = distinct !{[[LOOP8]], [[MP]]} diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html index c233171e63c811..0d796597d05c0e 100755 --- a/clang/www/cxx_status.html +++ b/clang/www/cxx_status.html @@ -187,7 +187,7 @@ <h2 id="cxx26">C++2c implementation status</h2> <tr> <td>Trivial infinite loops are not Undefined Behavior</td> <td><a href="https://wg21.link/P2809R3";>P2809R3</a> (<a href="#dr">DR</a>)</td> - <td class="none" align="center">No</td> + <td class="unreleased" align="center">Clang 19</td> </tr> <tr> <td>Erroneous behaviour for uninitialized reads</td> `````````` </details> https://github.com/llvm/llvm-project/pull/90066 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
