[PATCH] D118168: [LLVM] Introduce llvm.loop.finite metadata to represent loops which are known to iterate a finite number of times

[William Moses via Phabricator via llvm-commits]

wsmoses added a comment.

In D118168#3277051 <https://reviews.llvm.org/D118168#3277051>, @jdoerfert wrote:

> I think this is generally sensible but lacks tests and a clear path towards usage. Who will produce the annotation?

I think there are several target producers. For example, in here (https://github.com/llvm/llvm-project/issues/51103) an OpenMP for loop can produce it.



================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:1345
+bool isFinite(const Loop *L);
+
 /// Return true if this loop can be assumed to make progress.  (i.e. can't
----------------
jdoerfert wrote:
> Why do we need both, esp. given the Note?
For consistency with hasMustProgress? Happy to remove it if undesired.


================
Comment at: llvm/lib/Analysis/LoopInfo.cpp:1126
 
+static const char *LLVMLoopFinite = "llvm.loop.finite";
+
----------------
jdoerfert wrote:
> While it seems to be the state-of-the-art to duplicate the string all over the place, could we please start a new scheme in which we have this definition early in the file and used whenever we need the string. Feel free to update the other ones as well.
I concur, though I might prefer to split off the string restructuring from the langref change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118168