lebedev.ri added a comment. Some thoughts
================ Comment at: clang/lib/CodeGen/CGStmt.cpp:796 + bool EmitBoolCondBranch = !C || !C->isOne(); + bool CondIsConst = C; const SourceRange &R = S.getSourceRange(); ---------------- I think, if we really want to give this a name, perhaps we want something even more specific, perhaps `CondIsConstImm`/`CondIsConstInt`? ================ Comment at: clang/lib/CodeGen/CGStmt.cpp:801 + SourceLocToDebugLoc(R.getEnd()), + loopMustProgress(CondIsConst)); ---------------- Now this doesn't make sense. Why must loop progress if it's condition is a constant? I think this should be renamed? ================ Comment at: clang/lib/CodeGen/CodeGenFunction.h:508-521 if (CGM.getCodeGenOpts().getFiniteLoops() == CodeGenOptions::FiniteLoopsKind::Never) return false; + // C++11 and later guarantees that a thread eventually will do one of the + // following (6.9.2.3.1 in C++11): + // - terminate, ---------------- What happens for C++11+ when `-fno-finite-loops` is specified? I.e., shouldn't this be similar to `loopMustProgress()`? ================ Comment at: clang/lib/CodeGen/CodeGenFunction.h:508-521 if (CGM.getCodeGenOpts().getFiniteLoops() == CodeGenOptions::FiniteLoopsKind::Never) return false; + // C++11 and later guarantees that a thread eventually will do one of the + // following (6.9.2.3.1 in C++11): + // - terminate, ---------------- lebedev.ri wrote: > What happens for C++11+ when `-fno-finite-loops` is specified? > I.e., shouldn't this be similar to `loopMustProgress()`? I may be unaware, but to me this looks wrong. `getFiniteLoops()` should know the right answer always, we shouldn't need to do checks for standard versions here. ================ Comment at: clang/test/CodeGen/attr-mustprogress.c:8 // RUN: %clang_cc1 -std=c11 -fno-finite-loops -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=C99 %s int a = 0; ---------------- Here and for C++, i would suggest to have bullet-proof RUN line coverage: * -std=c99/-std=c11/-std=c18/-std=c2x * -std=c99/-std=c11/-std=c18/-std=c2x + -ffinite-loops * -std=c99/-std=c11/-std=c18/-std=c2x + -fno-finite-loops Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96418/new/ https://reviews.llvm.org/D96418 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits