Skip to content

Commit

Permalink
[clang] Refactor mustprogress handling, add it to all loops in c++11+.
Browse files Browse the repository at this point in the history 
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.

Reviewed By: aaron.ballman, lebedev.ri

Differential Revision: https://reviews.llvm.org/D96418
  • Loading branch information
@fhahn
fhahn committed Apr 30, 2021
1 parent 66b8a16 commit 6c31295
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 118 deletions.
48 changes: 14 additions & 34 deletions clang/lib/CodeGen/CGStmt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -816,20 +816,14 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,

// 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 CondIsConstInt = C != nullptr;
bool EmitBoolCondBranch = !CondIsConstInt || !C->isOne();
const SourceRange &R = S.getSourceRange();
LoopStack.push(LoopHeader.getBlock(), CGM.getContext(), CGM.getCodeGenOpts(),
WhileAttrs, SourceLocToDebugLoc(R.getBegin()),
SourceLocToDebugLoc(R.getEnd()), LoopMustProgress);
SourceLocToDebugLoc(R.getEnd()),
checkIfLoopMustProgress(CondIsConstInt));

// As long as the condition is true, go to the loop body.
llvm::BasicBlock *LoopBody = createBasicBlock("while.body");
Expand Down Expand Up @@ -920,20 +914,15 @@ void CodeGenFunction::EmitDoStmt(const DoStmt &S,

// "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 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()), LoopMustProgress);
SourceLocToDebugLoc(R.getEnd()),
checkIfLoopMustProgress(CondIsConstInt));

// As long as the condition is true, iterate the loop.
if (EmitBoolCondBranch) {
Expand Down Expand Up @@ -971,20 +960,15 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
llvm::BasicBlock *CondBlock = CondDest.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 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()), LoopMustProgress);
SourceLocToDebugLoc(R.getEnd()),
checkIfLoopMustProgress(CondIsConstInt));

// Create a cleanup scope for the condition variable cleanups.
LexicalScope ConditionScope(*this, S.getSourceRange());
Expand Down Expand Up @@ -1033,10 +1017,6 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
BoolCondVal = emitCondLikelihoodViaExpectIntrinsic(
BoolCondVal, Stmt::getLikelihood(S.getBody()));

if (llvm::ConstantInt *C = dyn_cast<llvm::ConstantInt>(BoolCondVal))
if (C->isOne())
FnIsMustProgress = false;

Builder.CreateCondBr(BoolCondVal, ForBody, ExitBlock, Weights);

if (ExitBlock != LoopExit.getBlock()) {
Expand Down
5 changes: 1 addition & 4 deletions clang/lib/CodeGen/CodeGenFunction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1186,17 +1186,14 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,

void CodeGenFunction::EmitFunctionBody(const Stmt *Body) {
incrementProfileCounter(Body);
if (CPlusPlusWithProgress())
FnIsMustProgress = true;

if (const CompoundStmt *S = dyn_cast<CompoundStmt>(Body))
EmitCompoundStmtWithoutScope(*S);
else
EmitStmt(Body);

// This is checked after emitting the function body so we know if there
// are any permitted infinite loops.
if (FnIsMustProgress)
if (checkIfFunctionMustProgress())
CurFn->addFnAttr(llvm::Attribute::MustProgress);
}

Expand Down
43 changes: 28 additions & 15 deletions clang/lib/CodeGen/CodeGenFunction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,35 +524,48 @@ class CodeGenFunction : public CodeGenTypeCache {
// applies to. nullptr if there is no 'musttail' on the current statement.
const CallExpr *MustTailCall = nullptr;

/// True if the current function should be marked mustprogress.
bool FnIsMustProgress = false;

/// True if the C++ Standard Requires Progress.
bool CPlusPlusWithProgress() {
/// Returns true if a function must make progress, which means the
/// mustprogress attribute can be added.
bool checkIfFunctionMustProgress() {
if (CGM.getCodeGenOpts().getFiniteLoops() ==
CodeGenOptions::FiniteLoopsKind::Never)
return false;

return getLangOpts().CPlusPlus11 || getLangOpts().CPlusPlus14 ||
getLangOpts().CPlusPlus17 || getLangOpts().CPlusPlus20;
// C++11 and later guarantees that a thread eventually will do one of the
// following (6.9.2.3.1 in C++11):
// - terminate,
// - make a call to a library I/O function,
// - perform an access through a volatile glvalue, or
// - perform a synchronization operation or an atomic operation.
//
// Hence each function is 'mustprogress' in C++11 or later.
return getLangOpts().CPlusPlus11;
}

/// True if the C Standard Requires Progress.
bool CWithProgress() {
/// 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;

return getLangOpts().C11 || getLangOpts().C17 || getLangOpts().C2x;
}
// 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;

/// True if the language standard requires progress in functions or
/// in infinite loops with non-constant conditionals.
bool LanguageRequiresProgress() {
return CWithProgress() || CPlusPlusWithProgress();
// Loops with non-constant conditions must make progress in C11 and later.
return getLangOpts().C11;
}

const CodeGen::CGBlockInfo *BlockInfo = nullptr;
Expand Down
31 changes: 23 additions & 8 deletions clang/test/CodeGen/attr-mustprogress.c
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
// RUN: %clang_cc1 -std=c89 -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=C99 %s
// RUN: %clang_cc1 -std=c99 -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=C99 %s
// RUN: %clang_cc1 -std=c11 -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=C11 %s
// RUN: %clang_cc1 -std=c18 -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=C11 %s
// RUN: %clang_cc1 -std=c2x -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=C11 %s
//
// Check -ffinite-loops option in combination with various standard versions.
// RUN: %clang_cc1 -std=c89 -ffinite-loops -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=FINITE %s
// RUN: %clang_cc1 -std=c99 -ffinite-loops -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=FINITE %s
// RUN: %clang_cc1 -std=c11 -ffinite-loops -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=FINITE %s
// RUN: %clang_cc1 -std=c18 -ffinite-loops -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=FINITE %s
// RUN: %clang_cc1 -std=c2x -ffinite-loops -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=FINITE %s
//
// Check -fno-finite-loops option in combination with various standard versions.
// RUN: %clang_cc1 -std=c89 -fno-finite-loops -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=C99 %s
// RUN: %clang_cc1 -std=c99 -fno-finite-loops -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=C99 %s
// 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
// RUN: %clang_cc1 -std=c18 -fno-finite-loops -triple=x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck --check-prefix=CHECK --check-prefix=C99 %s
Expand All @@ -25,7 +28,9 @@ int b = 0;
// CHECK-NEXT: entry:
// CHECK-NEXT: br label %for.cond
// CHECK: for.cond:
// CHECK-NOT: br {{.*}}!llvm.loop
// C99-NOT: br {{.*}}!llvm.loop
// C11-NOT: br {{.*}}!llvm.loop
// FINITE-NEXT: br {{.*}}!llvm.loop
//
void f0() {
for (; ;) ;
Expand All @@ -38,7 +43,9 @@ void f0() {
// CHECK: for.cond:
// CHECK-NEXT: br i1 true, label %for.body, label %for.end
// CHECK: for.body:
// CHECK-NOT: br {{.*}}, !llvm.loop
// C99-NOT: br {{.*}}, !llvm.loop
// C11-NOT: br {{.*}}, !llvm.loop
// FINITE-NEXT: br {{.*}}, !llvm.loop
// CHECK: for.end:
// CHECK-NEXT: ret void
//
Expand Down Expand Up @@ -75,7 +82,9 @@ void f2() {
// CHECK: for.cond:
// CHECK-NEXT: br i1 true, label %for.body, label %for.end
// CHECK: for.body:
// CHECK-NOT: br {{.*}}, !llvm.loop
// C99-NOT: br {{.*}}, !llvm.loop
// C11-NOT: br {{.*}}, !llvm.loop
// FINITE-NEXT: br {{.*}}, !llvm.loop
// CHECK: for.end:
// CHECK-NEXT: br label %for.cond1
// CHECK: for.cond1:
Expand All @@ -102,7 +111,9 @@ void F() {
// CHECK-NEXT: entry:
// CHECK-NEXT: br label %while.body
// CHECK: while.body:
// CHECK-NOT: br {{.*}}, !llvm.loop
// C99-NOT: br {{.*}}, !llvm.loop
// C11-NOT: br {{.*}}, !llvm.loop
// FINITE-NEXT: br {{.*}}, !llvm.loop
//
void w1() {
while (1) {
Expand Down Expand Up @@ -141,12 +152,14 @@ void w2() {
// CHECK-NEXT: br i1 [[CMP]], label %while.body, label %while.end
// CHECK: while.body:
// C99-NOT: br {{.*}} !llvm.loop
// C11: br label %while.cond, !llvm.loop [[LOOP4:!.*]]
// FINITE: br label %while.cond, !llvm.loop [[LOOP4:!.*]]
// C11-NEXT: br label %while.cond, !llvm.loop [[LOOP4:!.*]]
// FINITE-NEXT: br label %while.cond, !llvm.loop [[LOOP4:!.*]]
// CHECK: while.end:
// CHECK-NEXT: br label %while.body2
// CHECK: while.body2:
// CHECK-NOT: br {{.*}} !llvm.loop
// C99-NOT: br {{.*}} !llvm.loop
// C11-NOT: br {{.*}} !llvm.loop
// FINITE-NEXT: br {{.*}} !llvm.loop
//
void W() {
while (a == b) {
Expand All @@ -162,7 +175,9 @@ void W() {
// CHECK: do.body:
// CHECK-NEXT: br label %do.cond
// CHECK: do.cond:
// CHECK-NOT: br {{.*}}, !llvm.loop
// C99-NOT: br {{.*}}, !llvm.loop
// C11-NOT: br {{.*}}, !llvm.loop
// FINITE-NEXT: br {{.*}}, !llvm.loop
// CHECK: do.end:
// CHECK-NEXT: ret void
//
Expand Down
Loading

0 comments on commit 6c31295

Please sign in to comment.