lebedev.ri added a comment. In D94367#2489090 <https://reviews.llvm.org/D94367#2489090>, @fhahn wrote:
> 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. 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
