[PATCH] D86841: [clang] Add mustprogress and llvm.loop.mustprogress attribute deduction

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.


================
Comment at: clang/lib/CodeGen/CGStmt.cpp:801
+             getLangOpts().CPlusPlus11 || getLangOpts().CPlusPlus14 ||
+             getLangOpts().CPlusPlus17 || getLangOpts().C2x) {
+    MustProgress = true;
----------------
Also in C? And C2x in the end is probably CPLusPlus2x?


================
Comment at: clang/lib/CodeGen/CGStmt.cpp:894
+    else if (C->isOne())
+      IsMustProgress = false;
+  } else if (getLangOpts().C11 || getLangOpts().C17 || getLangOpts().C2x ||
----------------
Maybe call this FnIsMustProgress or the other one LoopMustProgress.


================
Comment at: clang/lib/CodeGen/CGStmt.cpp:898
+             getLangOpts().CPlusPlus17 || getLangOpts().C2x)
+    MustProgress = true;
+
----------------
same as above


================
Comment at: clang/lib/CodeGen/CGStmt.cpp:946
+       getLangOpts().CPlusPlus17 || getLangOpts().C2x) &&
+      (!S.getCond() || !S.getCond()->EvaluateAsInt(Result, getContext())))
+    MustProgress = true;
----------------
same as above.

Don't we have to update IsMustProgress here too?



================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1165
+    IsMustProgress = true;
+  }
+
----------------
no braces. why the mustprogress check?


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1172
+
+  if (IsMustProgress)
+    CurFn->addFnAttr(llvm::Attribute::MustProgress);
----------------
Say that we do this late because we need to see the body.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86841