[PATCH] D74209: [AssumeBundle] Add documentation for the operand bundles of an llvm.assume

[Florian Hahn via Phabricator via llvm-commits]

fhahn added reviewers: nlopes, reames, regehr, efriedma.
fhahn added a comment.

Added some additional reviewers, I think it would be good to have a few more people taking a look.



================
Comment at: llvm/docs/LangRef.rst:2115
+
+* The tag of the operand bundle is the attribute that can be assumed to hold.
+* The first argument if present is the value for which the attribute hold.
----------------
I think this list needs a bit of context, like 'An assume operand bundle has the form `"<tag>"(<holds for value>, <attribute argument>)`:
 * ....


Maybe also link to the section about attributes and clarify that only valid attributes are supported.


================
Comment at: llvm/docs/LangRef.rst:2117
+* The first argument if present is the value for which the attribute hold.
+* The Second argument if present is an argument of the attribute.
+
----------------
nit: second

also, what for attributes with more than 1 argument?


================
Comment at: llvm/docs/LangRef.rst:2119
+
+If there are no arguments the attribute is a property of the path.
+
----------------
Clarify which path?


================
Comment at: llvm/docs/LangRef.rst:2125
+
+      call void @llvm.assume(i1 true) ["align"(i32* %val, i32 8)]
+
----------------
jdoerfert wrote:
> jdoerfert wrote:
> > lebedev.ri wrote:
> > > It may be good to add a footnote that the arguments needn't be constants (or are they?)
> > I would prefer not to require that. As long as we come from `llvm::Attribute` that is the case but we derive information from other places (e.g., the user) eventually.
> Didn't read it properly. Good idea to say explicitly they might not be constant! Thx @lebedev.ri 
align isn't a string attribute I think. Shouldn't we use a non-string attribute here?


================
Comment at: llvm/docs/LangRef.rst:17581
 
+For operand bundles on an ``llvm.assume`` see
+:ref:`assume operand bundles <assume_opbundles>`.
----------------
nit: maybe say something like 'More complex assumptions can be encoded as :ref:`operand bundles <assume_opbundles>`.


================
Comment at: llvm/docs/LangRef.rst:17587
 
-The condition which the optimizer may assume is always true.
+The argument of the call is the condition which the optimizer may assume is
+always true.
----------------
Is this change needed?


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

https://reviews.llvm.org/D74209