[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-05 Thread Hubert Tong via cfe-commits


@@ -908,6 +908,69 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 =

hubert-reinterpretcast wrote:

> So... we treat it as a manifestly constant-evaluated for the purpose of 
> checking whether the loop is trivial, but then flips to not manifestly 
> constant-evaluated for the actual evaluation at runtime?

Yes.

> The wording could use some clarification...

The absence of wording to change the condition itself (that is, what is used at 
runtime) to be manifestly constant-evaluated is intentional. This wording is 
not unique. It is also used for the _determination_ of constant initialization, 
which #89565 (that you referred to) points out is also broken in Clang 
(however, in contrast to the condition case, once determined to be constant 
initialization, the initializer itself is considered manifestly 
constant-evaluated).

> Maybe we can run constant-evaluation before Sema::CheckForImmediateInvocation 
> runs, though.

I think that makes sense. The _constant-expression_ that we are evaluating 
introduces an immediate function context that suppresses immediate invocations 
(https://eel.is/c++draft/expr.const#16).

Example:
```cpp
struct A {
  constexpr A(const int ) : p() {}
  const int *p;
};
consteval A a() { return {42}; }
enum { X = (a(), 0) }; // OK, no immediate invocations; Clang and GCC both 
accept
```

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-05 Thread Eli Friedman via cfe-commits


@@ -908,6 +908,69 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 =

efriedma-quic wrote:

So... we treat it as a manifestly constant-evaluated for the purpose of 
checking whether the loop is trivial, but then flips to not manifestly 
constant-evaluated for the actual evaluation at runtime?  The wording could use 
some clarification...

Due to the way Sema::CheckForImmediateInvocation works, once we decide an 
expression is not manifestly constant-evaluated, we can't actually 
constant-evaluate it, I think; the AST is modified.  Maybe we can run 
constant-evaluation before Sema::CheckForImmediateInvocation runs, though.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-04 Thread Hubert Tong via cfe-commits


@@ -908,6 +908,69 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 =

hubert-reinterpretcast wrote:

> As a standards-wording issue, the standard has to do something here

This was extensively discussed in CWG. The wording makes the evaluation for the 
purposes of determining whether the loop is trivially infinite a manifestly 
constant-evaluated context. It leaves the normal evaluation as not a manifestly 
constant-evaluated context.

The operative words are "interpreted as a _constant-expression_" in 
https://eel.is/c++draft/stmt.iter.general#3.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-04 Thread Eli Friedman via cfe-commits


@@ -908,6 +908,69 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 =

efriedma-quic wrote:

As a standards-wording issue, the standard has to do *something* here: it 
doesn't define what it means to constant-evaluate something in a context that 
isn't manifestly constant-evaluated.

As a practical matter, it's very unlikely the precise wording the standard uses 
actually matters, sure.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-04 Thread via cfe-commits


@@ -908,6 +908,69 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 =

cor3ntin wrote:

>  I guess we could also say that if the condition would evaluate to false if 
> we treat it as manifestly constant-evaluated, it's not manifestly 
> constant-evaluated. So we'd amend [expr.const]p20 to say something like: "An 
> expression or conversion is manifestly constant-evaluated if it is [...] the 
> condition of trivially empty iteration statement, is a constant expression 
> when interpreted as a constant-expression, and the constant expression would 
> evaluate to true".

Do we have _any_ motivation for doing so?
In addition of being extremely mind-bendy, there is no practical application.


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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-03 Thread Eli Friedman via cfe-commits


@@ -908,6 +908,69 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 =

efriedma-quic wrote:

In any case, addressing this probably doesn't require modifying any of the code 
in this patch; the change would be to make Sema introduce a ConstantExpr when 
appropriate.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-03 Thread Eli Friedman via cfe-commits


@@ -908,6 +908,69 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 =

efriedma-quic wrote:

I guess we could also say that if the condition would evaluate to false if we 
treat it as manifestly constant-evaluated, it's not manifestly 
constant-evaluated.  So we'd amend [expr.const]p20 to say something like: "An 
expression or conversion is manifestly constant-evaluated if it is [...] the 
condition of trivially empty iteration statement, is a constant expression when 
interpreted as a constant-expression, and the constant expression would 
evaluate to true".

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-03 Thread Eli Friedman via cfe-commits

https://github.com/efriedma-quic edited 
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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-03 Thread Eli Friedman via cfe-commits


@@ -908,6 +908,69 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 =

efriedma-quic wrote:

One case to consider:

```
int main() {
  while (!__builtin_is_constant_evaluated()) {}
}
```

If you treat the condition of the loop as manifestly constant-evaluated, the 
loop is finite: the condition evaluates to false.  If we say it's not 
manifestly constant-evaluated, but do some sort of constant-evaluation anyway, 
it's a well-defined infinite loop.  If we somehow treat it as manifestly 
constant-evaluated for the constant evaluation, but not at runtime, it's UB.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-03 Thread Eli Friedman via cfe-commits


@@ -908,6 +908,69 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 =

efriedma-quic wrote:

is_constant_evaluated is equivalent to if consteval, i.e. it's true in a 
context which is manifestly constant-evaluated.  So... is the condition 
manifestly constant-evaluated in this case?  If it is, that's fine, I guess, 
but the standard definition of "manifestly constant-evaluated" should probably 
be amended to reflect that.  Otherwise, I'm not sure how to interpret the 
standard here; whether an expression is manifestly constant-evaluated changes 
the way it's represented in the AST.

See also #89565.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-03 Thread Hubert Tong via cfe-commits


@@ -908,6 +908,69 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 =

hubert-reinterpretcast wrote:

The following does not have UB according to the standard, but Clang treats it 
as UB:
```
int main(void) {
  int x = 1;
  while (__builtin_is_constant_evaluated() || x);
  return 0;
}
```

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-03 Thread Hubert Tong via cfe-commits

https://github.com/hubert-reinterpretcast commented:

@cor3ntin, can you take a look at the case I added? Thanks.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-03 Thread Hubert Tong via cfe-commits

https://github.com/hubert-reinterpretcast edited 
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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-03 Thread via cfe-commits

https://github.com/cor3ntin closed 
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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-02 Thread via cfe-commits

cor3ntin wrote:

@erichkeane @efriedma-quic Thanks for the review, I'll probably land that 
tomorrow.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-02 Thread Eli Friedman via cfe-commits

https://github.com/efriedma-quic approved this pull request.

LGTM

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-02 Thread via cfe-commits


@@ -908,6 +908,72 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 CondIsTrue = CondIsConstInt && (!ControllingExpression ||
+   Result.Val.getInt().getBoolValue());
+
+  if (getLangOpts().C99 && CondIsConstInt)

cor3ntin wrote:

This turns out to just not be useful at all (but harmless) because we end up 
landing at the end of the function (and return false there)

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-02 Thread via cfe-commits


@@ -908,6 +908,72 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 CondIsTrue = CondIsConstInt && (!ControllingExpression ||
+   Result.Val.getInt().getBoolValue());
+
+  if (getLangOpts().C99 && CondIsConstInt)

cor3ntin wrote:

> Where did getLangOpts().C99 come from? I think the old code treated 
> c89/c99/c++98 as equivalent (never must-progress).

Good catch. that is supposed to be C89. I guess C && !c11

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-02 Thread Eli Friedman via cfe-commits


@@ -908,6 +908,72 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool HasEmptyBody) {
+  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 CondIsTrue = CondIsConstInt && (!ControllingExpression ||
+   Result.Val.getInt().getBoolValue());
+
+  if (getLangOpts().C99 && CondIsConstInt)

efriedma-quic wrote:

Where did getLangOpts().C99 come from?  I think the old code treated 
c89/c99/c++98 as equivalent (never must-progress).

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-05-01 Thread via cfe-commits


@@ -908,6 +908,74 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool IsTrivialCXXLoop) {
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==
+  CodeGenOptions::FiniteLoopsKind::Never)
+return false;
+
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==
+  CodeGenOptions::FiniteLoopsKind::Always &&
+  !getLangOpts().CPlusPlus11)

cor3ntin wrote:

Done!

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-30 Thread Eli Friedman via cfe-commits


@@ -908,6 +908,74 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool IsTrivialCXXLoop) {
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==
+  CodeGenOptions::FiniteLoopsKind::Never)
+return false;
+
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==
+  CodeGenOptions::FiniteLoopsKind::Always &&
+  !getLangOpts().CPlusPlus11)

efriedma-quic wrote:

There are basically four modes we have logic for here:

- No mustprogress optimizations
- C11 mustprogress
- C++23 mustprogress
- always mustprogress

I don't think there's really much reason to keep "always mustprogress" mode; 
like you note, it's not really useful.  Let's just make -ffinite-loops mean the 
new C++ rules.

Of course, -fno-finite-loops means no mustprogress.

This does mean we don't provide a way to explicitly access C11 mustprogress 
mode, I guess; it's not equivalent to either of -ffinite-loops or 
-fno-finite-loops.  But not sure how much we care.  We could add a flag 
-fc11-finite-loops if someone does care.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-30 Thread via cfe-commits


@@ -908,6 +908,74 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool IsTrivialCXXLoop) {
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==
+  CodeGenOptions::FiniteLoopsKind::Never)
+return false;
+
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==
+  CodeGenOptions::FiniteLoopsKind::Always &&
+  !getLangOpts().CPlusPlus11)

cor3ntin wrote:

Yes, I wasn't sure what to do here. Does it make sense to override the 
standard? but given the loop is for sure infinite, it could be spelled `--ub`.
On the other hand, if we override all modes, is the flag useful at all?

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-30 Thread Eli Friedman via cfe-commits


@@ -908,6 +908,74 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool IsTrivialCXXLoop) {
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==
+  CodeGenOptions::FiniteLoopsKind::Never)
+return false;
+
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==
+  CodeGenOptions::FiniteLoopsKind::Always &&
+  !getLangOpts().CPlusPlus11)

efriedma-quic wrote:

If we're going to define -ffinite-loops to allow trivial infinite loops, might 
as well do it in all modes, not just C++11 mode?

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-30 Thread via cfe-commits

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff e50a857fb16bcfe7cfc99bf87db620bc82d1cff5 
043132f8c5bbc00d647dedb873e47c5dda905f9b -- clang/lib/CodeGen/CGStmt.cpp 
clang/lib/CodeGen/CodeGenFunction.cpp clang/lib/CodeGen/CodeGenFunction.h 
clang/test/CodeGenCXX/attr-mustprogress.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index a3c66e433b..3a86ae0a1f 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -914,8 +914,9 @@ bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
   CodeGenOptions::FiniteLoopsKind::Never)
 return false;
 
-  if(CGM.getCodeGenOpts().getFiniteLoops() == 
CodeGenOptions::FiniteLoopsKind::Always
-  && !getLangOpts().CPlusPlus11)
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==
+  CodeGenOptions::FiniteLoopsKind::Always &&
+  !getLangOpts().CPlusPlus11)
 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
@@ -928,8 +929,8 @@ bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
   (ControllingExpression->EvaluateAsInt(Result, getContext()) &&
Result.Val.isInt());
 
-  bool CondIsTrue = CondIsConstInt &&
-(!ControllingExpression || Result.Val.getInt().getBoolValue());
+  bool CondIsTrue = CondIsConstInt && (!ControllingExpression ||
+   Result.Val.getInt().getBoolValue());
 
   if (getLangOpts().C99 && CondIsConstInt)
 return false;
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index 3196d05906..546beae4af 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1471,7 +1471,8 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, 
llvm::Function *Fn,
 
   // Ensure that the function adheres to the forward progress guarantee, which
   // is required by certain optimizations.
-  // In C++11 and up, the attribute will be removed if the body contains a 
trivial empty loop.
+  // In C++11 and up, the attribute will be removed if the body contains a
+  // trivial empty loop.
   if (checkIfFunctionMustProgress())
 CurFn->addFnAttr(llvm::Attribute::MustProgress);
 

``




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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-30 Thread Erich Keane via cfe-commits

https://github.com/erichkeane approved this pull request.

This LGTM, but please give @efriedma-quic a day or two to confirm this looks 
right for him too.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-30 Thread via cfe-commits

yronglin wrote:

> @yronglin I'm reluctant to do that, that would be testing the optimizer in 
> the front end. If the patch did not fix this bug, there would be a back end 
> bug that should be fixed there

Agree!

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-30 Thread via cfe-commits

cor3ntin wrote:

@yronglin I'm reluctant to do that, that would be testing the optimizer in the 
front end.
If the patch did not fix this bug, there would be a back end bug that should be 
fixed there

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread via cfe-commits

yronglin wrote:

I'd like to add the test case from the paper:

```
#include 

int main() {
  while (true)
; 
}

void unreachable() {
  std::cout << "Hello world!" << std::endl;
}
```

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread via cfe-commits


@@ -908,6 +908,74 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+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 CondIsTrue = 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 && CondIsTrue) {
+  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  static bool hasEmptyLoopBody(const LoopStmt ) {
+  if constexpr (std::is_same_v) {
+if (S.getInc())
+  return false;
+  }
+  const Stmt *Body = S.getBody();
+  if (!Body || isa(Body))
+return true;
+  if (const CompoundStmt *Compound = dyn_cast(Body))

cor3ntin wrote:

I added some tests (and test for false conditions, which are not trivial in c++)

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread Shafik Yaghmour via cfe-commits


@@ -908,6 +908,74 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+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 CondIsTrue = 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 && CondIsTrue) {
+  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  static bool hasEmptyLoopBody(const LoopStmt ) {
+  if constexpr (std::is_same_v) {
+if (S.getInc())
+  return false;
+  }
+  const Stmt *Body = S.getBody();
+  if (!Body || isa(Body))
+return true;
+  if (const CompoundStmt *Compound = dyn_cast(Body))

shafik wrote:

It does not look like the test cover the `CompoundStmt` case.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread Shafik Yaghmour via cfe-commits


@@ -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.

shafik wrote:

```suggestion
  // In C++11 and forward, the attribute will be removed if the body contains a 
trivial empty loop.
```

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread via cfe-commits


@@ -908,6 +908,73 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+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);

cor3ntin wrote:

Thanks for checking. I did revert the patch I made this morning :)

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread Eli Friedman via cfe-commits


@@ -908,6 +908,73 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+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);

efriedma-quic wrote:

I... have to apologize here.  I was pretty sure I remembered how mustprogress 
worked, but then I went back to read the patches, and it's actually slightly 
different than what I remembered. mustprogress applies to control flow and 
calls within a function, but it doesn't propagate: an infinite loop in a 
non-mustprogress counts as an "effect".  I'll propose a patch to clarify 
LangRef.

So your original patch was essentially right.  If we drop the mustprogress 
attribute from functions that contain an infinite loop, we end up with 
essentially the right behavior: if a function doesn't contain a 
trivially-infinite loop, we can mark it mustprogress.  And we mark individual 
loops with metadata either way.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread via cfe-commits

https://github.com/cor3ntin updated 
https://github.com/llvm/llvm-project/pull/90066

>From a89ed3064119d110da2289e78bb85630342a2b18 Mon Sep 17 00:00:00 2001
From: Corentin Jabot 
Date: Thu, 25 Apr 2024 16:34:29 +0200
Subject: [PATCH 1/5] [Clang] Implement P2809: Trivial infinite loops are not
 Undefined Behavior

https://wg21.link/P2809R3
---
 clang/docs/ReleaseNotes.rst |  4 +-
 clang/lib/CodeGen/CGStmt.cpp| 81 ++---
 clang/lib/CodeGen/CodeGenFunction.cpp   |  1 +
 clang/lib/CodeGen/CodeGenFunction.h | 23 +-
 clang/test/CodeGenCXX/attr-mustprogress.cpp | 70 --
 clang/www/cxx_status.html   |  2 +-
 6 files changed, 107 insertions(+), 74 deletions(-)

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 `_.
 
-- Implemented `P2448R2: Relaxing some constexpr restrictions 
`_.
+- Implemented `P2809R3: Trivial infinite loops are not Undefined Behavior 
`_.
 
 C++2c Feature Support
 ^
@@ -131,6 +131,8 @@ C++2c Feature Support
 
 - Implemented `P2573R2: = delete("should have a reason"); 
`_
 
+- Implemented `P2573R2: = delete("should have a reason"); 
`_
+
 
 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 ) {
 incrementProfileCounter();
 }
 
+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  static bool hasEmptyLoopBody(const LoopStmt ) {
+  if constexpr (std::is_same_v) {
+if (S.getInc())
+  return false;
+  }
+  const Stmt *Body = S.getBody();
+  if (!Body || isa(Body))
+return true;
+  if (const CompoundStmt *Compound = dyn_cast(Body))
+return Compound->body_empty();
+  return false;
+}
+
 void CodeGenFunction::EmitWhileStmt(const WhileStmt ,
 ArrayRef WhileAttrs) {
   // Emit the header for the loop, which will also become
@@ -942,13 +1009,12 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt ,
   // while(1) is common, avoid extra exit blocks.  Be sure
   // to correctly handle break/continue though.
   llvm::ConstantInt *C = dyn_cast(BoolCondVal);
-  bool CondIsConstInt = C != nullptr;
-  bool EmitBoolCondBranch = !CondIsConstInt || !C->isOne();
+  bool EmitBoolCondBranch = !C || !C->isOne();
   const SourceRange  = S.getSourceRange();
   LoopStack.push(LoopHeader.getBlock(), CGM.getContext(), CGM.getCodeGenOpts(),
  WhileAttrs, 

[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread via cfe-commits


@@ -122,15 +122,15 @@ C++23 Feature Support
   materialize temporary object which is a prvalue in discarded-value 
expression.
 - Implemented `P1774R8: Portable assumptions `_.
 
-- Implemented `P2448R2: Relaxing some constexpr restrictions 
`_.

cor3ntin wrote:

Missed that, thanks

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread via cfe-commits


@@ -122,15 +122,15 @@ C++23 Feature Support
   materialize temporary object which is a prvalue in discarded-value 
expression.
 - Implemented `P1774R8: Portable assumptions `_.
 
-- Implemented `P2448R2: Relaxing some constexpr restrictions 
`_.

yronglin wrote:

I don't think we should remove this.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread via cfe-commits

https://github.com/cor3ntin updated 
https://github.com/llvm/llvm-project/pull/90066

>From a89ed3064119d110da2289e78bb85630342a2b18 Mon Sep 17 00:00:00 2001
From: Corentin Jabot 
Date: Thu, 25 Apr 2024 16:34:29 +0200
Subject: [PATCH 1/4] [Clang] Implement P2809: Trivial infinite loops are not
 Undefined Behavior

https://wg21.link/P2809R3
---
 clang/docs/ReleaseNotes.rst |  4 +-
 clang/lib/CodeGen/CGStmt.cpp| 81 ++---
 clang/lib/CodeGen/CodeGenFunction.cpp   |  1 +
 clang/lib/CodeGen/CodeGenFunction.h | 23 +-
 clang/test/CodeGenCXX/attr-mustprogress.cpp | 70 --
 clang/www/cxx_status.html   |  2 +-
 6 files changed, 107 insertions(+), 74 deletions(-)

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 `_.
 
-- Implemented `P2448R2: Relaxing some constexpr restrictions 
`_.
+- Implemented `P2809R3: Trivial infinite loops are not Undefined Behavior 
`_.
 
 C++2c Feature Support
 ^
@@ -131,6 +131,8 @@ C++2c Feature Support
 
 - Implemented `P2573R2: = delete("should have a reason"); 
`_
 
+- Implemented `P2573R2: = delete("should have a reason"); 
`_
+
 
 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 ) {
 incrementProfileCounter();
 }
 
+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  static bool hasEmptyLoopBody(const LoopStmt ) {
+  if constexpr (std::is_same_v) {
+if (S.getInc())
+  return false;
+  }
+  const Stmt *Body = S.getBody();
+  if (!Body || isa(Body))
+return true;
+  if (const CompoundStmt *Compound = dyn_cast(Body))
+return Compound->body_empty();
+  return false;
+}
+
 void CodeGenFunction::EmitWhileStmt(const WhileStmt ,
 ArrayRef WhileAttrs) {
   // Emit the header for the loop, which will also become
@@ -942,13 +1009,12 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt ,
   // while(1) is common, avoid extra exit blocks.  Be sure
   // to correctly handle break/continue though.
   llvm::ConstantInt *C = dyn_cast(BoolCondVal);
-  bool CondIsConstInt = C != nullptr;
-  bool EmitBoolCondBranch = !CondIsConstInt || !C->isOne();
+  bool EmitBoolCondBranch = !C || !C->isOne();
   const SourceRange  = S.getSourceRange();
   LoopStack.push(LoopHeader.getBlock(), CGM.getContext(), CGM.getCodeGenOpts(),
  WhileAttrs, 

[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread via cfe-commits

https://github.com/cor3ntin updated 
https://github.com/llvm/llvm-project/pull/90066

>From a89ed3064119d110da2289e78bb85630342a2b18 Mon Sep 17 00:00:00 2001
From: Corentin Jabot 
Date: Thu, 25 Apr 2024 16:34:29 +0200
Subject: [PATCH 1/3] [Clang] Implement P2809: Trivial infinite loops are not
 Undefined Behavior

https://wg21.link/P2809R3
---
 clang/docs/ReleaseNotes.rst |  4 +-
 clang/lib/CodeGen/CGStmt.cpp| 81 ++---
 clang/lib/CodeGen/CodeGenFunction.cpp   |  1 +
 clang/lib/CodeGen/CodeGenFunction.h | 23 +-
 clang/test/CodeGenCXX/attr-mustprogress.cpp | 70 --
 clang/www/cxx_status.html   |  2 +-
 6 files changed, 107 insertions(+), 74 deletions(-)

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 `_.
 
-- Implemented `P2448R2: Relaxing some constexpr restrictions 
`_.
+- Implemented `P2809R3: Trivial infinite loops are not Undefined Behavior 
`_.
 
 C++2c Feature Support
 ^
@@ -131,6 +131,8 @@ C++2c Feature Support
 
 - Implemented `P2573R2: = delete("should have a reason"); 
`_
 
+- Implemented `P2573R2: = delete("should have a reason"); 
`_
+
 
 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 ) {
 incrementProfileCounter();
 }
 
+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  static bool hasEmptyLoopBody(const LoopStmt ) {
+  if constexpr (std::is_same_v) {
+if (S.getInc())
+  return false;
+  }
+  const Stmt *Body = S.getBody();
+  if (!Body || isa(Body))
+return true;
+  if (const CompoundStmt *Compound = dyn_cast(Body))
+return Compound->body_empty();
+  return false;
+}
+
 void CodeGenFunction::EmitWhileStmt(const WhileStmt ,
 ArrayRef WhileAttrs) {
   // Emit the header for the loop, which will also become
@@ -942,13 +1009,12 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt ,
   // while(1) is common, avoid extra exit blocks.  Be sure
   // to correctly handle break/continue though.
   llvm::ConstantInt *C = dyn_cast(BoolCondVal);
-  bool CondIsConstInt = C != nullptr;
-  bool EmitBoolCondBranch = !CondIsConstInt || !C->isOne();
+  bool EmitBoolCondBranch = !C || !C->isOne();
   const SourceRange  = S.getSourceRange();
   LoopStack.push(LoopHeader.getBlock(), CGM.getContext(), CGM.getCodeGenOpts(),
  WhileAttrs, 

[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread via cfe-commits

https://github.com/cor3ntin updated 
https://github.com/llvm/llvm-project/pull/90066

>From a89ed3064119d110da2289e78bb85630342a2b18 Mon Sep 17 00:00:00 2001
From: Corentin Jabot 
Date: Thu, 25 Apr 2024 16:34:29 +0200
Subject: [PATCH 1/2] [Clang] Implement P2809: Trivial infinite loops are not
 Undefined Behavior

https://wg21.link/P2809R3
---
 clang/docs/ReleaseNotes.rst |  4 +-
 clang/lib/CodeGen/CGStmt.cpp| 81 ++---
 clang/lib/CodeGen/CodeGenFunction.cpp   |  1 +
 clang/lib/CodeGen/CodeGenFunction.h | 23 +-
 clang/test/CodeGenCXX/attr-mustprogress.cpp | 70 --
 clang/www/cxx_status.html   |  2 +-
 6 files changed, 107 insertions(+), 74 deletions(-)

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 `_.
 
-- Implemented `P2448R2: Relaxing some constexpr restrictions 
`_.
+- Implemented `P2809R3: Trivial infinite loops are not Undefined Behavior 
`_.
 
 C++2c Feature Support
 ^
@@ -131,6 +131,8 @@ C++2c Feature Support
 
 - Implemented `P2573R2: = delete("should have a reason"); 
`_
 
+- Implemented `P2573R2: = delete("should have a reason"); 
`_
+
 
 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 ) {
 incrementProfileCounter();
 }
 
+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  static bool hasEmptyLoopBody(const LoopStmt ) {
+  if constexpr (std::is_same_v) {
+if (S.getInc())
+  return false;
+  }
+  const Stmt *Body = S.getBody();
+  if (!Body || isa(Body))
+return true;
+  if (const CompoundStmt *Compound = dyn_cast(Body))
+return Compound->body_empty();
+  return false;
+}
+
 void CodeGenFunction::EmitWhileStmt(const WhileStmt ,
 ArrayRef WhileAttrs) {
   // Emit the header for the loop, which will also become
@@ -942,13 +1009,12 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt ,
   // while(1) is common, avoid extra exit blocks.  Be sure
   // to correctly handle break/continue though.
   llvm::ConstantInt *C = dyn_cast(BoolCondVal);
-  bool CondIsConstInt = C != nullptr;
-  bool EmitBoolCondBranch = !CondIsConstInt || !C->isOne();
+  bool EmitBoolCondBranch = !C || !C->isOne();
   const SourceRange  = S.getSourceRange();
   LoopStack.push(LoopHeader.getBlock(), CGM.getContext(), CGM.getCodeGenOpts(),
  WhileAttrs, 

[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-25 Thread Eli Friedman via cfe-commits


@@ -908,6 +908,73 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+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);

efriedma-quic wrote:

The mustprogress function attribute it affects not only loops, but also 
loop-like constructs (goto/musttail/etc.).  And that effect infects the whole 
program: if a program's main() function is mustprogress, the rest of the 
program is implicitly mustprogress.

I think it's reasonable to turn off the function attribute everywhere, and just 
mark loops.  That's a bit more conservative than the C++ standard rules (which 
also allow treating non-loop constructs as mustprogress), but in practice most 
of the benefit we get from mustprogress comes from marking loops.  And it makes 
the emitted IR more similar between C and C++, so optimizations will impact 
both more consistently.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-25 Thread via cfe-commits


@@ -908,6 +908,73 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+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);

cor3ntin wrote:

We do mark the loops `mustprogress` in C++ too.

I'm not sure how either of these attributes work, so I've try to preserve 
existing behavior.
IE, a function will be marked  `mustprogress` in C++11+ _unless_ at least one 
loop is not `mustprogress`.

If you telling me that the `mustprogress` attribute has no effect in the 
absence of loops in the body, or when all the loops are mustprogress, maybe we 
should never use it in C++11+ modes?


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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-25 Thread Eli Friedman via cfe-commits


@@ -908,6 +908,73 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+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);

efriedma-quic wrote:

This doesn't have the effect you want.  Even if you remove mustprogress from 
the current function, we'll re-infer it if the caller is mustprogress.

You have two options here, I think:

- Introduce a call to llvm.sideeffect() (which dodges the whole issue because 
it counts as a "side-effect" for mustprogress).
- Remove the mustprogress attribute from everything, and just mark individual 
loops with mustprogress metadata, like we do in C.

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-25 Thread via cfe-commits


@@ -131,6 +131,8 @@ C++2c Feature Support
 
 - Implemented `P2573R2: = delete("should have a reason"); 
`_
 
+- Implemented `P2573R2: = delete("should have a reason"); 
`_

cor3ntin wrote:

Good question!

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-25 Thread Erich Keane via cfe-commits


@@ -131,6 +131,8 @@ C++2c Feature Support
 
 - Implemented `P2573R2: = delete("should have a reason"); 
`_
 
+- Implemented `P2573R2: = delete("should have a reason"); 
`_

erichkeane wrote:

What is going on here?

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-25 Thread Erich Keane via cfe-commits


@@ -908,6 +908,73 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool IsTrivialCXXLoop) {
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==

erichkeane wrote:

Is this not reversed?  If a loop is finite, I thought it is NOT `MustProgress`?

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-25 Thread Erich Keane via cfe-commits


@@ -908,6 +908,73 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+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 &&

erichkeane wrote:

```suggestion
  bool CondIsTrue = CondIsConstInt &&
```

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-25 Thread Erich Keane via cfe-commits


@@ -908,6 +908,73 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool IsTrivialCXXLoop) {
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==

erichkeane wrote:

Hrm... I see this is exsting code, but this confuses me/is reverse from what I 
thought.  

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


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-25 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang-codegen

Author: cor3ntin (cor3ntin)


Changes

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 `_.
 
-- Implemented `P2448R2: Relaxing some constexpr restrictions 
`_.
+- Implemented `P2809R3: Trivial infinite loops are not Undefined Behavior 
`_.
 
 C++2c Feature Support
 ^
@@ -131,6 +131,8 @@ C++2c Feature Support
 
 - Implemented `P2573R2: = delete("should have a reason"); 
`_
 
+- Implemented `P2573R2: = delete("should have a reason"); 
`_
+
 
 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 ) {
 incrementProfileCounter();
 }
 
+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  static bool hasEmptyLoopBody(const LoopStmt ) {
+  if constexpr (std::is_same_v) {
+if (S.getInc())
+  return false;
+  }
+  const Stmt *Body = S.getBody();
+  if (!Body || isa(Body))
+return true;
+  if (const CompoundStmt *Compound = dyn_cast(Body))
+return Compound->body_empty();
+  return false;
+}
+
 void CodeGenFunction::EmitWhileStmt(const WhileStmt ,
 ArrayRef WhileAttrs) {
   // Emit the header for the loop, which will also become
@@ -942,13 +1009,12 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt ,
   // while(1) is common, avoid extra exit blocks.  Be sure
   // to correctly handle break/continue though.
   llvm::ConstantInt *C = dyn_cast(BoolCondVal);
-  bool CondIsConstInt = C != nullptr;
-  bool EmitBoolCondBranch = !CondIsConstInt || !C->isOne();
+  bool EmitBoolCondBranch = !C || !C->isOne();
   const SourceRange  = S.getSourceRange();
   LoopStack.push(LoopHeader.getBlock(), 

[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-25 Thread via cfe-commits

https://github.com/cor3ntin created 
https://github.com/llvm/llvm-project/pull/90066

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.

>From a89ed3064119d110da2289e78bb85630342a2b18 Mon Sep 17 00:00:00 2001
From: Corentin Jabot 
Date: Thu, 25 Apr 2024 16:34:29 +0200
Subject: [PATCH] [Clang] Implement P2809: Trivial infinite loops are not
 Undefined Behavior

https://wg21.link/P2809R3
---
 clang/docs/ReleaseNotes.rst |  4 +-
 clang/lib/CodeGen/CGStmt.cpp| 81 ++---
 clang/lib/CodeGen/CodeGenFunction.cpp   |  1 +
 clang/lib/CodeGen/CodeGenFunction.h | 23 +-
 clang/test/CodeGenCXX/attr-mustprogress.cpp | 70 --
 clang/www/cxx_status.html   |  2 +-
 6 files changed, 107 insertions(+), 74 deletions(-)

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 `_.
 
-- Implemented `P2448R2: Relaxing some constexpr restrictions 
`_.
+- Implemented `P2809R3: Trivial infinite loops are not Undefined Behavior 
`_.
 
 C++2c Feature Support
 ^
@@ -131,6 +131,8 @@ C++2c Feature Support
 
 - Implemented `P2573R2: = delete("should have a reason"); 
`_
 
+- Implemented `P2573R2: = delete("should have a reason"); 
`_
+
 
 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 ) {
 incrementProfileCounter();
 }
 
+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  static bool hasEmptyLoopBody(const LoopStmt ) {
+  if constexpr (std::is_same_v) {
+if (S.getInc())
+  return false;
+  }
+  const Stmt *Body = S.getBody();
+  if (!Body || isa(Body))
+return true;
+  if (const CompoundStmt *Compound = dyn_cast(Body))
+return Compound->body_empty();
+  return false;
+}
+
 void CodeGenFunction::EmitWhileStmt(const WhileStmt ,
 ArrayRef WhileAttrs) {
   // Emit the header for the loop, which will also become
@@ -942,13 +1009,12 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt ,
   // while(1) is common, avoid extra exit blocks.  Be sure
   // to correctly handle break/continue though.
   llvm::ConstantInt *C = dyn_cast(BoolCondVal);
-  bool CondIsConstInt =