[PATCH] D79454: [IR] `byval` arguments cause reads at call sites

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 6 17:04:25 PDT 2020


efriedma added a comment.

I think it's a fair argument that readnone/etc. should only apply to memory operations actually written in the callee, not the implicit byval memory operation (which is "on the call edge").  But I don't think the existing semantics fail in any important respect.  The rules are self-consistent, straightforward to explain, and are reasonably convenient for optimizations.  And really, any result we arrive at is going to feel sort of weird: outside of byval, there usually aren't any reads to user-visible memory between the call instruction and the first instruction of the callee, and byval is the one exception.  I agree your proposed model is slightly better, but I'm not enthusiastic about changing this because the incentive to change it is weak.

(Also, I guess this makes mayReadFromMemory() slightly slower, but that's probably not going to lead to any visible difference.)

The other concern is that it's backward-compatible.  I think we don't need to do anything special here; we were previously being conservative, and not adding readnone to calls with byval arguments, so old IR should just work, I think.

Please stage this in the correct order: first fix LangRef, then change existing optimizations to respect byval properly, then add your new optimization to Attributor.

> EDIT: We might actually loose out if we prevent readnone in the presence of byval arguments if, the user provided the readnone/ony annotation

I think you can get similar results with argmemonly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79454





More information about the llvm-commits mailing list