[PATCH] D151943: [InstCombine] Propagate some func/arg/ret attributes from caller to callsite

Thu Jun 1 15:12:05 PDT 2023

goldstein.w.n added a comment.

In D151943#4388993 <https://reviews.llvm.org/D151943#4388993>, @nikic wrote:

>> This patch only implements the attributes can can be blindly propagated.
>
> At least the function-level nofree and memory attributes cannot be propagated. E.g. a function that allocates and frees internally is nofree. Writes and reads to allocas are `memory(none)` from a function perspective, etc. You can see multiple miscompiles due to this in the test diffs.

Will drop nofree.

Ahh, think the memory attrs would also apply for malloc/free memory even if malloc/free is local to function so can apply if all arguments are non-alloca base. If all args non-alloca base then can apply to full function.

> More generally, I'm not sure this code belongs in InstCombine.

I somewhat agree although I'm not sure where else? I thought about putting it in the attributor but 1) the attributor is not enabled (and its not clear when/if that will happen) and 2) the attributor seems to be for IPO which doesn't really describe this well imo. Figured since its an exact transform on the specific instructions for the sake of other optimization passes instcombine fits the mold reasonable well.

================
Comment at: llvm/test/Transforms/InstCombine/AMDGPU/memcpy-from-constant.ll:217
-; CHECK-NEXT:    call void @llvm.memmove.p0.p4.i32(ptr noundef nonnull align 4 dereferenceable(16) [[SCRATCH:%.*]], ptr addrspace(4) noundef align 4 dereferenceable(16) [[ARG:%.*]], i32 16, i1 false)
 ; CHECK-NEXT:    ret void
 ;
----------------
The reason for these being deleted is the functions are marked `readnone`. I think that attribute is probably a mistake but don't think this is buggy to delete.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151943