[PATCH] D158081: [IR] Add writable attribute

Fri Aug 18 11:53:53 PDT 2023

efriedma added inline comments.

================
Comment at: llvm/test/Transforms/LICM/scalar-promote.ll:889
+define void @writable_cond_store(ptr sret(i32) noalias writable %ptr) {
+; CHECK-LABEL: @writable_cond_store(
 ; CHECK-NEXT:    [[PTR_PROMOTED:%.*]] = load i32, ptr [[PTR:%.*]], align 4
----------------
nikic wrote:
> efriedma wrote:
> > I'm not sure this transform is actually legal without `dereferenceable(4)`?  The way "writable" is specified, no "deferenceable" means no phantom store, so this is introducing a race.
> `sret(%T)` implies `dereferenceable(sizeof(%T))`. I should probably tweak the wording to not talk about `dereferenceable(N)` specifically but attributes implying dereferenceability in general.
> 
> (And we should really drop the type argument from `sret` and make people specify `align` and `dereferenceable` instead...)
Is that actually the way the compiler is reasoning about it?  Can you add a negative test showing the sret is actually necessary here?

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

https://reviews.llvm.org/D158081