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

[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 <https://reviews.llvm.org/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