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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Looks better, I went over again and commented on minor language stuff. I'm not an expert either ;)

@fhahn  what do you think?



================
Comment at: llvm/docs/LangRef.rst:2119
+
+If there is no arguments the attribute is a property of the path
+
----------------
jdoerfert wrote:
> Nit: `are no arguments`, dot missing at the end.
It's not the path arguably but the (single) location of the `llvm.assume`


================
Comment at: llvm/docs/LangRef.rst:2113
+This allows representing assumptions that are either hard or not possible to
+represent as a boolean argument of an :ref:`llvm.assume <int_assume>`.
+
----------------
`allows representing` -> `allow representing`

`Operand bundles enable assumptions that are either hard or impossible to represent as a boolean argument of`


================
Comment at: llvm/docs/LangRef.rst:2134
+
+allows the optimizer to assume that the path is cold.
+
----------------
path -> call location


================
Comment at: llvm/docs/LangRef.rst:2136
+
+If any of the provided guarantees are incorrect the behavior is undefined.
+
----------------
are incorrect -> are violated at runtime
we might also reference the regular assume here.


================
Comment at: llvm/docs/LangRef.rst:2139
+Even if the property can be represented as a boolean property like ``nonnull``.
+Using operand bundles to express the property still have benefits:
+
----------------
Even if the assumed property can be encoded as a boolean value like ``nonnull``.

has benefits   *or*   can still have benefits


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

https://reviews.llvm.org/D74209