[PATCH] D74209: [AssumeBundle] Add documentation for assume operand bundles

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Overall the test looks fine. I added a few comments, some rewording proposals and minor comments. We also need a proper commit message ;)

@fhahn Please take a look :)



================
Comment at: llvm/docs/LangRef.rst:2113
+This allows representing assumptions that are either hard or not possible to
+represent via a boolean check.
+
----------------
Nit: Maybe say: `as a boolean argument of an `llvm.assume`.


================
Comment at: llvm/docs/LangRef.rst:2119
+
+If there is no arguments the attribute is a property of the path
+
----------------
Nit: `are no arguments`, dot missing at the end.


================
Comment at: llvm/docs/LangRef.rst:2145
+  not have every canonicalizations to deduce the property from any arbitrary
+  expressions that check the property.
+* Expressing the property using operand bundles make detecting that the value
----------------
It's a bit abstract. Maybe something along the lines of:

```
Encoding attributes as operand bundles removes the need for an instruction sequence that represents the property, e.g., `icmp ne i32* %p, null` for `nonnull`. These sequences can
have negative effects due to the additional uses they introduce and the potential for them
to be changed or combined with surrounding user code.
```


================
Comment at: llvm/docs/LangRef.rst:2148
+  is used in an :ref:`llvm.assume <int_assume>` easier. Which can minimize the
+  impact of adding a use to the value.
+
----------------
"make detecting that the value is used in an :ref:`llvm.assume <int_assume>` easier."
->
"makes it easy to identify uses in an :ref:`llvm.assume <int_assume>`. This then simplifies and improves heuristics, e.g., for use "use-sensitive" optimizations."


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

https://reviews.llvm.org/D74209