[PATCH] D158081: [IR] Add writable attribute

[Eli Friedman via Phabricator via llvm-commits]

efriedma 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.
----------------
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.


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

https://reviews.llvm.org/D158081