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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
