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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

In D74209#1876692 <https://reviews.llvm.org/D74209#1876692>, @nlopes wrote:

> LGTM overall, just some nitpicks.
>  Something I would add is a link to the list of attributes (is this `function-attributes`?).


LGTM as well with some minor suggestions, and one clarification for @nlopes, below.
I think with the modifications we can go ahead and improve as we go.

The first paragraph has the word `attribute`. We can link to the two attribute lang ref sections (as we have both parameter and function attributes). Maybe:

... ​assumptions that a :ref:`function attribute <fnattrs>` or :ref:`parameter attribute <paramattrs>` holds ...



================
Comment at: llvm/docs/LangRef.rst:2126
+
+If there are no arguments the attribute is a property of the call location.
+
----------------
nlopes wrote:
> which call location?
of the ``llvm.assume`` call location.


================
Comment at: llvm/docs/LangRef.rst:2134
+
+allows the optimizer to assume that at this point ``%val`` has an alignment of
+at least 8 and ``%ptr`` has an alignment of at least ``%alignment``.
----------------
nlopes wrote:
> nitpick, maybe at this point -> from this point forward
> since the assumptions hold for the rest of the program.
> nitpick, maybe at this point -> from this point forward
> since the assumptions hold for the rest of the program.

Arguably not the ones that do not have an associated value, e.g., the `cold()` example below. All the ones that "bind" to a value do hold for the rest of the program though. But since that is "implied", and we have the other ones, I would not go there for now.


================
Comment at: llvm/docs/LangRef.rst:2141
+
+allows the optimizer to assume that the call location is cold and that ``%val``
+may not be null.
----------------
nlopes wrote:
> which call location? The one that returns `%cond` or `%val`?  Should be made explicit.
Again, we need to say: the ``llvm.assume`` call location


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

https://reviews.llvm.org/D74209