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

[Tyker via Phabricator via llvm-commits]

Tyker marked an inline comment as done.
Tyker added inline comments.


================
Comment at: llvm/docs/LangRef.rst:2125-2128
+      call void @llvm.assume(i1 true) ["align"(i32* %val, i32 8)]
+
+allows the optimizer to assume that ``%val`` has an alignment of at least 8
+at this point.
----------------
jdoerfert wrote:
> Tyker wrote:
> > Tyker wrote:
> > > fhahn wrote:
> > > > 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?
> > > i edited an example so it is not always a constant is it clear enough this way ?
> > align isn't a string attribute but tags in operand bundles need to be a string. so all attributes are encoded with there name as a strings.
> It's especially interesting for attributes that take only a constant in their attribute for, e.g., align. Maybe extend the example to show both non-constant values for those and how tags can occur multiple times:
> `call void @llvm.assume(i1 true) ["align"(i32* %val, i32 8), "align"(i32* %ptr, i32 %alignment)]`
it is possible to represent non-constant arguments using operand bundles. but the current query API doesn't handle that case. it is possible to deal with it but i think this will be at the cost of worse API and some performance.

i think this should be disallowed at least for now.


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

https://reviews.llvm.org/D74209