[PATCH] D158081: [IR] Add writable attribute

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/docs/LangRef.rst:1537
+    This attribute is only meaningful in conjunction with ``dereferenceable(N)``
+    or another attribute that implies the first ``N`` bytes of the pointer
+    argument are dereferenceable.
----------------
efriedma wrote:
> nikic wrote:
> > efriedma wrote:
> > > I think I'd suggest dropping the "or another attribute" part; it makes it harder to reason about the semantics on the caller side.
> > My motivation for this is to allow reusing standard dereferenceability APIs. Otherwise we would have to thread an extra option through APIs like isDereferenceablePointer() that restricts it to only dereferenceable and excludes the other attributes.
> Value::getPointerDereferenceableBytes() isn't that complicated; we can write out the other cases it handles here, I guess.  I'm more worried about it being open-ended.
> 
> To be more specific about what I'm worried about, the exact number of bytes written is relevant for a couple of reasons.  One, alias analysis in the caller: ideally, the caller should be able to understand which bytes are modified (although I guess that might require attributes that don't currently exist).  And two, any transform that modifies a relevant attribute needs to worry about whether the transform invalidates the "writable" attribute.
I've added an explicit list of the attributes.

I think in terms of inference the case to be concerned about is when `dereferenceable` with a larger byte range is inferred (maybe Attributor does that, haven't checked). The rest are ABI attributes and we shouldn't infer those.

How would the caller use `writable` for alias analysis?


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

https://reviews.llvm.org/D158081