subject:"\[PATCH\] D94367\: \[Clang\]\[Driver\] Add \-ffinite\-loops flags"

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

2021-02-04 Thread Florian Hahn via Phabricator via cfe-commits

fhahn added a comment.

In D94367#2542809 , @atmnpatel wrote:

> Wait actually, we're gonna need D94366  
> anyways, I'll get address @xbolva00's comments by eod.

FWIW I think we should sort this one (D94367 ) 
first, because it may simplify D94366 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94367/new/

https://reviews.llvm.org/D94367

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

2021-02-04 Thread Atmn Patel via Phabricator via cfe-commits

atmnpatel added a comment.

Wait actually, we're gonna need D94366  
anyways, I'll get address @xbolva00's comments by eod.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94367/new/

https://reviews.llvm.org/D94367

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

2021-02-04 Thread Atmn Patel via Phabricator via cfe-commits

atmnpatel added a comment.

@fhahn  Sorry for the hold up, I don't think I'll get to this relatively soon, 
I'll abandon this one and D94366  to sweep the 
board.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94367/new/

https://reviews.llvm.org/D94367

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

2021-02-04 Thread Florian Hahn via Phabricator via cfe-commits

fhahn added a comment.
Herald added a reviewer: jansvoboda11.

@atmnpatel reverse ping. Just curious if you think you'll have time to follow 
up on this patch soonish?

Otherwise I'm probably going to add at least the `-fno-finite-loops` option, 
because we have users which code breaks due to the `mustprogress` loop deletion 
changes (because of problems in the source code, but they need a way to disable 
until they can fix all sources)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94367/new/

https://reviews.llvm.org/D94367

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

2021-01-10 Thread Dávid Bolvanský via Phabricator via cfe-commits

xbolva00 added a comment.

Just note: gcc enables -ffinite-loops with opt level -O2 and higher.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94367/new/

https://reviews.llvm.org/D94367

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

2021-01-10 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

In D94367#2489090 , @fhahn wrote:

> Thanks for putting up the patch!
>
> I think the code here and  D94366  could be 
> simplified if we would introduce 2 codegen options to distinguish the 
> differences between C and C++:
>
> - `functions-must-progress=true/false`: C++ behavior >= c++11; all threads 
> must make forward progress, we should be able to mark all functions as 
> `mustprogress` and there should be no need to mark the individual loops as 
> `mustprogress`.
> - `loops-must-progress=all/none/c11`: `all` -> all loops are marked 
> `mustprogress`, `none` -> no loops are marked, `c11` -> all loops that do not 
> match the C11 escape hatch are marked `mustprogress`.
>
> Now
>
> - `c++11` and above, `functions-must-progress=true`, 
> `loops-must-progress=none`,
> - `c11` and above `functions-must-progress=false`, `loops-must-progress=c11`,
> - `-ffinite-loops` -> `loops-must-progress=all`,
> - `-fnofinite-loops` -> , `functions-must-progress=false`, 
> `loops-must-progress=false`
>
> In CodeGen, we always add `mustprogress` to functions exactly if 
> `functions-must-progress=true`. For loops, we could have a helper 
> `shouldMarkLoopAsMustProgress(bool C11Exception)`, which returns true 
> depending on `loops-must-progress` and `C11Exceptions`, which the caller sets 
> to true if the loop condition is one that is allowed for infinite loops in 
> `C11`.
>
> This should allow us to remove `CGF::FnIsMustProgress` and the code to update 
> it; now either all functions have `mustprogress` or none have, depending on 
> `functions-must-progress`. We still need `CGLoopInfo::MustProgress`, but we 
> only need to update it depending on `loops-must-progress`,
>
> I think this would overall simplify the handling in `Codegen`, but it's very 
> possible that I am missing something from earlier discussions. I am also 
> adding a few additional people who may have additional thought.
>
> One things that's slightly odd with the proposal is that for C++, 
> `loops-must-progress` would be `none`, which has the potential to be a bit 
> confusing. We could make this clear in the documentation or just set it to 
> `all`, although as I mentioned in the beginning, that should not really be 
> necessary. It might be helpful/necessary if loops inlined from C++ function 
> inside C functions should keep their `mustprogress` property.

What happens when inlining `mustprogress` function into non-`mustprogress` 
function (or more generally, cross-language LTO)?
We'd need to ensure that in such cases the loops get annotated as 
`mustprogress`,
so they don't loose that status, which is why i think it may be better
to always annotate loops, even if their functions already are `mustprogress`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94367/new/

https://reviews.llvm.org/D94367

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

2021-01-10 Thread Florian Hahn via Phabricator via cfe-commits

fhahn added reviewers: rjmccall, aaron.ballman, erichkeane.
fhahn added a comment.

Thanks for putting up the patch!

I think the code here and  D94366  could be 
simplified if we would introduce 2 codegen options to distinguish the 
differences between C and C++:

- `functions-must-progress=true/false`: C++ behavior >= c++11; all threads must 
make forward progress, we should be able to mark all functions as 
`mustprogress` and there should be no need to mark the individual loops as 
`mustprogress`.
- `loops-must-progress=all/none/c11`: `all` -> all loops are marked 
`mustprogress`, `none` -> no loops are marked, `c11` -> all loops that do not 
match the C11 escape hatch are marked `mustprogress`.

Now

- `c++11` and above, `functions-must-progress=true`, `loops-must-progress=none`,
- `c11` and above `functions-must-progress=false`, `loops-must-progress=c11`,
- `-ffinite-loops` -> `loops-must-progress=all`,
- `-fnofinite-loops` -> , `functions-must-progress=false`, 
`loops-must-progress=false`

In CodeGen, we always add `mustprogress` to functions exactly if 
`functions-must-progress=true`. For loops, we could have a helper 
`shouldMarkLoopAsMustProgress(bool C11Exception)`, which returns true depending 
on `loops-must-progress` and `C11Exceptions`, which the caller sets to true if 
the loop condition is one that is allowed for infinite loops in `C11`.

This should allow us to remove `CGF::FnIsMustProgress` and the code to update 
it; now either all functions have `mustprogress` or none have, depending on 
`functions-must-progress`. We still need `CGLoopInfo::MustProgress`, but we 
only need to update it depending on `loops-must-progress`,

I think this would overall simplify the handling in `Codegen`, but it's very 
possible that I am missing something from earlier discussions. I am also adding 
a few additional people who may have additional thought.

One things that's slightly odd with the proposal is that for C++, 
`loops-must-progress` would be `none`, which has the potential to be a bit 
confusing. We could make this clear in the documentation or just set it to 
`all`, although as I mentioned in the beginning, that should not really be 
necessary. It might be helpful/necessary if loops inlined from C++ function 
inside C functions should keep their `mustprogress` property.




Comment at: clang/docs/ClangCommandLineReference.rst:2290
+
+Assume that all loops terminate or never assume that they do. This takes 
precedence over the language standard.
+

It looks like the options are mostly ordered. Could you move it to the right 
place?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94367/new/

https://reviews.llvm.org/D94367

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

2021-01-09 Thread Atmn Patel via Phabricator via cfe-commits

atmnpatel updated this revision to Diff 315641.
atmnpatel added a comment.

Update CommandLineReference.rst to also include `-fno-finite-loops`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94367/new/

https://reviews.llvm.org/D94367

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/finite-loops.c
  clang/test/CodeGenCXX/finite-loops.cpp
  clang/test/Driver/clang_f_opts.c

Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -52,6 +52,15 @@
 // CHECK-REROLL-LOOPS: "-freroll-loops"
 // CHECK-NO-REROLL-LOOPS-NOT: "-freroll-loops"
 
+// RUN: %clang -### -S -ffinite-loops %s 2>&1 | FileCheck -check-prefix=CHECK-FINITE-LOOPS %s
+// RUN: %clang -### -S -fno-finite-loops %s 2>&1 | FileCheck -check-prefix=CHECK-NO-FINITE-LOOPS %s
+// RUN: %clang -### -S -fno-finite-loops -ffinite-loops %s 2>&1 | FileCheck -check-prefix=CHECK-FINITE-LOOPS %s
+// RUN: %clang -### -S -ffinite-loops -fno-finite-loops %s 2>&1 | FileCheck -check-prefix=CHECK-NO-FINITE-LOOPS %s
+// CHECK-FINITE-LOOPS: "-ffinite-loops"
+// CHECK-FINITE-LOOPS-NOT: "-fno-finite-loops"
+// CHECK-NO-FINITE-LOOPS: "-fno-finite-loops"
+// CHECK-NO-FINITE-LOOPS-NOT: "-ffinite-loops"
+
 // RUN: %clang -### -S -fprofile-sample-accurate %s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-SAMPLE-ACCURATE %s
 // CHECK-PROFILE-SAMPLE-ACCURATE: "-fprofile-sample-accurate"
 
@@ -292,6 +301,7 @@
 // RUN: -malign-functions=100 \
 // RUN: -malign-loops=100 \
 // RUN: -malign-jumps=100 \
+// RUN: -ffinite-loops -fno-finite-loops  \
 // RUN: %s 2>&1 | FileCheck --check-prefix=IGNORE %s
 // IGNORE-NOT: error: unknown argument
 
Index: clang/test/CodeGenCXX/finite-loops.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/finite-loops.cpp
@@ -0,0 +1,99 @@
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -std=c++03 -o - %s | FileCheck %s -check-prefix=CHECK-CPP03-LOOPS
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -std=c++03 -ffinite-loops -o - %s | FileCheck %s -check-prefix=CHECK-FINITE-LOOPS
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -std=c++03 -fno-finite-loops -o - %s | FileCheck %s -check-prefix=CHECK-INFINITE-LOOPS
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -std=c++11 -o - %s | FileCheck %s -check-prefix=CHECK-CPP11-LOOPS
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -std=c++11 -ffinite-loops -o - %s | FileCheck %s -check-prefix=CHECK-FINITE-LOOPS
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -std=c++11 -fno-finite-loops -o - %s | FileCheck %s -check-prefix=CHECK-INFINITE-LOOPS
+
+// CHECK-CPP03-LOOPS: Function Attrs: noinline nounwind optnone
+// CHECK-CPP03-LOOPS-LABEL: @_Z1fii(
+// CHECK-CPP03-LOOPS-NOT:{{.*}} !llvm.loop !
+//
+// CHECK-FINITE-LOOPS: Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-FINITE-LOOPS-LABEL: @_Z1fii(
+// CHECK-FINITE-LOOPS:{{.*}} !llvm.loop !
+// CHECK-FINITE-LOOPS:{{.*}} !llvm.loop !
+//
+// CHECK-INFINITE-LOOPS: Function Attrs: noinline nounwind optnone
+// CHECK-INFINITE-LOOPS-LABEL: @_Z1fii(
+// CHECK-INFINITE-LOOPS-NOT:{{.*}} !llvm.loop !
+//
+// CHECK-CPP11-LOOPS: Function Attrs: noinline nounwind optnone
+// CHECK-CPP11-LOOPS-LABEL: @_Z1fii(
+// CHECK-CPP11-LOOPS:{{.*}} !llvm.loop !
+// CHECK-CPP11-LOOPS:{{.*}} !llvm.loop !
+//
+int f(int a, int b) {
+  for (; a != b; ) {
+if (a == b)
+  return 1;
+  }
+
+  for (;;) {
+if (a != b)
+  return 1;
+  }
+  return 0;
+}
+
+// CHECK-CPP03-LOOPS: Function Attrs: noinline nounwind optnone
+// CHECK-CPP03-LOOPS-LABEL: @_Z2dwii(
+// CHECK-CPP03-LOOPS-NOT:{{.*}} !llvm.loop !
+//
+// CHECK-FINITE-LOOPS: Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-FINITE-LOOPS-LABEL: @_Z2dwii(
+// CHECK-FINITE-LOOPS:{{.*}} !llvm.loop !
+// CHECK-FINITE-LOOPS:{{.*}} !llvm.loop !
+//
+// CHECK-INFINITE-LOOPS: Function Attrs: noinline nounwind optnone
+// CHECK-INFINITE-LOOPS-LABEL: @_Z2dwii(
+// CHECK-INFINITE-LOOPS-NOT:{{.*}} !llvm.loop !
+//
+// CHECK-CPP11-LOOPS: Function Attrs: noinline nounwind optnone
+// CHECK-CPP11-LOOPS-LABEL: @_Z2dwii(
+// CHECK-CPP11-LOOPS:{{.*}} !llvm.loop !
+// CHECK-CPP11-LOOPS:{{.*}} !llvm.loop !
+//
+int dw(int a, int b) {
+  do {
+if (a == b)
+  return 1;
+  } while (a != b);
+
+  do {
+if (a != b)
+  return 1;
+  } while (1);
+  return 0;
+}
+
+//

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

2021-01-09 Thread Atmn Patel via Phabricator via cfe-commits

atmnpatel created this revision.
atmnpatel added reviewers: fhahn, jdoerfert, xbolva00.
Herald added subscribers: dexonsmith, dang.
atmnpatel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Following D94366 , clang will strictly adheres 
to the language standard
when deciding whether or not to emit `mustprogress` attributes for
loops. This patch adds two flags: `-ffinite-loops` and
`-fno-finite-loops` that override the language standard into either
never emitting `mustprogress` attributes or emitting `mustprogress`
attributes for all loops/functions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94367

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/finite-loops.c
  clang/test/CodeGenCXX/finite-loops.cpp
  clang/test/Driver/clang_f_opts.c

Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -52,6 +52,15 @@
 // CHECK-REROLL-LOOPS: "-freroll-loops"
 // CHECK-NO-REROLL-LOOPS-NOT: "-freroll-loops"
 
+// RUN: %clang -### -S -ffinite-loops %s 2>&1 | FileCheck -check-prefix=CHECK-FINITE-LOOPS %s
+// RUN: %clang -### -S -fno-finite-loops %s 2>&1 | FileCheck -check-prefix=CHECK-NO-FINITE-LOOPS %s
+// RUN: %clang -### -S -fno-finite-loops -ffinite-loops %s 2>&1 | FileCheck -check-prefix=CHECK-FINITE-LOOPS %s
+// RUN: %clang -### -S -ffinite-loops -fno-finite-loops %s 2>&1 | FileCheck -check-prefix=CHECK-NO-FINITE-LOOPS %s
+// CHECK-FINITE-LOOPS: "-ffinite-loops"
+// CHECK-FINITE-LOOPS-NOT: "-fno-finite-loops"
+// CHECK-NO-FINITE-LOOPS: "-fno-finite-loops"
+// CHECK-NO-FINITE-LOOPS-NOT: "-ffinite-loops"
+
 // RUN: %clang -### -S -fprofile-sample-accurate %s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-SAMPLE-ACCURATE %s
 // CHECK-PROFILE-SAMPLE-ACCURATE: "-fprofile-sample-accurate"
 
@@ -292,6 +301,7 @@
 // RUN: -malign-functions=100 \
 // RUN: -malign-loops=100 \
 // RUN: -malign-jumps=100 \
+// RUN: -ffinite-loops -fno-finite-loops  \
 // RUN: %s 2>&1 | FileCheck --check-prefix=IGNORE %s
 // IGNORE-NOT: error: unknown argument
 
Index: clang/test/CodeGenCXX/finite-loops.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/finite-loops.cpp
@@ -0,0 +1,99 @@
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -std=c++03 -o - %s | FileCheck %s -check-prefix=CHECK-CPP03-LOOPS
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -std=c++03 -ffinite-loops -o - %s | FileCheck %s -check-prefix=CHECK-FINITE-LOOPS
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -std=c++03 -fno-finite-loops -o - %s | FileCheck %s -check-prefix=CHECK-INFINITE-LOOPS
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -std=c++11 -o - %s | FileCheck %s -check-prefix=CHECK-CPP11-LOOPS
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -std=c++11 -ffinite-loops -o - %s | FileCheck %s -check-prefix=CHECK-FINITE-LOOPS
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -std=c++11 -fno-finite-loops -o - %s | FileCheck %s -check-prefix=CHECK-INFINITE-LOOPS
+
+// CHECK-CPP03-LOOPS: Function Attrs: noinline nounwind optnone
+// CHECK-CPP03-LOOPS-LABEL: @_Z1fii(
+// CHECK-CPP03-LOOPS-NOT:{{.*}} !llvm.loop !
+//
+// CHECK-FINITE-LOOPS: Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-FINITE-LOOPS-LABEL: @_Z1fii(
+// CHECK-FINITE-LOOPS:{{.*}} !llvm.loop !
+// CHECK-FINITE-LOOPS:{{.*}} !llvm.loop !
+//
+// CHECK-INFINITE-LOOPS: Function Attrs: noinline nounwind optnone
+// CHECK-INFINITE-LOOPS-LABEL: @_Z1fii(
+// CHECK-INFINITE-LOOPS-NOT:{{.*}} !llvm.loop !
+//
+// CHECK-CPP11-LOOPS: Function Attrs: noinline nounwind optnone
+// CHECK-CPP11-LOOPS-LABEL: @_Z1fii(
+// CHECK-CPP11-LOOPS:{{.*}} !llvm.loop !
+// CHECK-CPP11-LOOPS:{{.*}} !llvm.loop !
+//
+int f(int a, int b) {
+  for (; a != b; ) {
+if (a == b)
+  return 1;
+  }
+
+  for (;;) {
+if (a != b)
+  return 1;
+  }
+  return 0;
+}
+
+// CHECK-CPP03-LOOPS: Function Attrs: noinline nounwind optnone
+// CHECK-CPP03-LOOPS-LABEL: @_Z2dwii(
+// CHECK-CPP03-LOOPS-NOT:{{.*}} !llvm.loop !
+//
+// CHECK-FINITE-LOOPS: Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-FINITE-LOOPS-LABEL: @_Z2dwii(
+// CHECK-FINITE-LOOPS:{{.*}} !llvm.loop !
+// CHECK-FINITE-LOOPS:{{.*}} !llvm.loop !
+//
+// CHECK-INFINITE-LOOPS: Function Attrs: noinline nounwind optnone
+// CHECK-INFINITE-LOOPS-LABEL:

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

9 matches

Site Navigation

Mail list logo

Footer information