[PATCH] D115003: [funcattrs] Infer writeonly argument attribute [part 2]

[Philip Reames via Phabricator via cfe-commits]

reames added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:728
+      } else if (CB.hasFnAttr(Attribute::WriteOnly) ||
+                 CB.dataOperandHasImpliedAttr(UseIndex, Attribute::WriteOnly)) {
+        IsWrite = true;
----------------
nikic wrote:
> This could be written as `CB.doesNotReadMemory() || CB.doesNotReadMemory(UseIndex)` for symmetry with the other cases. Though personally I find the naming of these methods really unfortunate (they should probably be called `onlyWritesMemory()`) so maybe the explicit form is better.
I'm leaving this as is, but I agree we could use some naming cleanup here.  (I may even do it since this is the second time I've hit this.)

Though to highlight "does not read memory" and "WriteOnly" are not quite the same.  The former allows readnone which is correct, but surprising here.  Personally, I hate subtle semantic surprises and am increasing of the view that we should check the attributes we mean.  (At least in inference code.)


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

https://reviews.llvm.org/D115003