From 0b09313cd53316eacbdc5e98d4ef00bef2c41d02 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Tue, 4 Jan 2022 08:39:51 -0800 Subject: [PATCH] [funcattrs] Infer writeonly argument attribute [part 2] This builds on the code from D114963, and extends it to handle calls both direct and indirect. With the revised code structure (from series of previously landed NFCs), this is pretty straight forward. One thing to note is that we can not infer writeonly for arguments which might be captured. If the pointer can be read back by the caller, and then read through, we have no way to track that. This is the same restriction we have for readonly, except that we get no mileage out of the "callee can be readonly" exception since a writeonly param on a readonly function is either a) readnone or b) UB. This means we can't actually infer much unless nocapture has already been inferred. Differential Revision: https://reviews.llvm.org/D115003 --- clang/test/CodeGen/arm-vfp16-arguments.c | 2 +- clang/test/CodeGenCXX/wasm-args-returns.cpp | 2 +- .../CodeGenOpenCL/amdgpu-abi-struct-coerce.cl | 6 +++--- llvm/lib/Transforms/IPO/FunctionAttrs.cpp | 12 ++++++++--- .../TypeBasedAliasAnalysis/functionattrs.ll | 2 +- llvm/test/Other/cgscc-devirt-iteration.ll | 2 +- .../Transforms/FunctionAttrs/norecurse.ll | 2 +- .../Transforms/FunctionAttrs/writeonly.ll | 20 +++++++++++++------ 8 files changed, 31 insertions(+), 17 deletions(-) diff --git a/clang/test/CodeGen/arm-vfp16-arguments.c b/clang/test/CodeGen/arm-vfp16-arguments.c index e11ec1508bbf..0ad099092a9a 100644 --- a/clang/test/CodeGen/arm-vfp16-arguments.c +++ b/clang/test/CodeGen/arm-vfp16-arguments.c @@ -71,6 +71,6 @@ void test_hfa(hfa_t a) {} hfa_t ghfa; hfa_t test_ret_hfa(void) { return ghfa; } -// CHECK-SOFT: define{{.*}} void @test_ret_hfa(%struct.hfa_t* noalias nocapture sret(%struct.hfa_t) align 8 %agg.result) +// CHECK-SOFT: define{{.*}} void @test_ret_hfa(%struct.hfa_t* noalias nocapture writeonly sret(%struct.hfa_t) align 8 %agg.result) // CHECK-HARD: define{{.*}} arm_aapcs_vfpcc [2 x <2 x i32>] @test_ret_hfa() // CHECK-FULL: define{{.*}} arm_aapcs_vfpcc %struct.hfa_t @test_ret_hfa() diff --git a/clang/test/CodeGenCXX/wasm-args-returns.cpp b/clang/test/CodeGenCXX/wasm-args-returns.cpp index c05bb44c05a3..d71bb28eabcc 100644 --- a/clang/test/CodeGenCXX/wasm-args-returns.cpp +++ b/clang/test/CodeGenCXX/wasm-args-returns.cpp @@ -30,7 +30,7 @@ struct two_fields { double d, e; }; test(two_fields); -// CHECK: define void @_Z7forward10two_fields(%struct.two_fields* noalias nocapture sret(%struct.two_fields) align 8 %{{.*}}, %struct.two_fields* nocapture readonly byval(%struct.two_fields) align 8 %{{.*}}) +// CHECK: define void @_Z7forward10two_fields(%struct.two_fields* noalias nocapture writeonly sret(%struct.two_fields) align 8 %{{.*}}, %struct.two_fields* nocapture readonly byval(%struct.two_fields) align 8 %{{.*}}) // // CHECK: define void @_Z15test_two_fieldsv() // CHECK: %[[tmp:.*]] = alloca %struct.two_fields, align 8 diff --git a/clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl b/clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl index 17333cc80e14..350bb3c69366 100644 --- a/clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl +++ b/clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl @@ -403,14 +403,14 @@ struct_arr16 func_ret_struct_arr16() return s; } -// CHECK: define{{.*}} void @func_ret_struct_arr32(%struct.struct_arr32 addrspace(5)* noalias nocapture sret(%struct.struct_arr32) align 4 %agg.result) +// CHECK: define{{.*}} void @func_ret_struct_arr32(%struct.struct_arr32 addrspace(5)* noalias nocapture writeonly sret(%struct.struct_arr32) align 4 %agg.result) struct_arr32 func_ret_struct_arr32() { struct_arr32 s = { 0 }; return s; } -// CHECK: define{{.*}} void @func_ret_struct_arr33(%struct.struct_arr33 addrspace(5)* noalias nocapture sret(%struct.struct_arr33) align 4 %agg.result) +// CHECK: define{{.*}} void @func_ret_struct_arr33(%struct.struct_arr33 addrspace(5)* noalias nocapture writeonly sret(%struct.struct_arr33) align 4 %agg.result) struct_arr33 func_ret_struct_arr33() { struct_arr33 s = { 0 }; @@ -468,7 +468,7 @@ double_nested_struct func_double_nested_struct_ret(int4 arg0, int arg1) { // CHECK: define{{.*}} void @func_large_struct_padding_arg_direct(i8 %arg.coerce0, i32 %arg.coerce1, i8 %arg.coerce2, i32 %arg.coerce3, i8 %arg.coerce4, i8 %arg.coerce5, i16 %arg.coerce6, i16 %arg.coerce7, [3 x i8] %arg.coerce8, i64 %arg.coerce9, i32 %arg.coerce10, i8 %arg.coerce11, i32 %arg.coerce12, i16 %arg.coerce13, i8 %arg.coerce14) void func_large_struct_padding_arg_direct(large_struct_padding arg) { } -// CHECK: define{{.*}} void @func_large_struct_padding_arg_store(%struct.large_struct_padding addrspace(1)* nocapture %out, %struct.large_struct_padding addrspace(5)* nocapture readonly byval(%struct.large_struct_padding) align 8 %arg) +// CHECK: define{{.*}} void @func_large_struct_padding_arg_store(%struct.large_struct_padding addrspace(1)* nocapture writeonly %out, %struct.large_struct_padding addrspace(5)* nocapture readonly byval(%struct.large_struct_padding) align 8 %arg) void func_large_struct_padding_arg_store(global large_struct_padding* out, large_struct_padding arg) { *out = arg; } diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp index 8fb0c2dc7613..bc3c3da44729 100644 --- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp +++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp @@ -720,10 +720,16 @@ determinePointerAccessAttrs(Argument *A, // The accessors used on call site here do the right thing for calls and // invokes with operand bundles. - if (!CB.onlyReadsMemory() && !CB.onlyReadsMemory(UseIndex)) - return Attribute::None; - if (!CB.doesNotAccessMemory(UseIndex)) + if (CB.doesNotAccessMemory(UseIndex)) { + /* nop */ + } else if (CB.onlyReadsMemory() || CB.onlyReadsMemory(UseIndex)) { IsRead = true; + } else if (CB.hasFnAttr(Attribute::WriteOnly) || + CB.dataOperandHasImpliedAttr(UseIndex, Attribute::WriteOnly)) { + IsWrite = true; + } else { + return Attribute::None; + } break; } diff --git a/llvm/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll b/llvm/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll index 65c0e27d5f46..e0c9e45101f3 100644 --- a/llvm/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll +++ b/llvm/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll @@ -49,7 +49,7 @@ define void @test2_yes(i8* %p, i8* %q, i64 %n) nounwind { ret void } -; CHECK: define void @test2_no(i8* nocapture %p, i8* nocapture readonly %q, i64 %n) #5 { +; CHECK: define void @test2_no(i8* nocapture writeonly %p, i8* nocapture readonly %q, i64 %n) #5 { define void @test2_no(i8* %p, i8* %q, i64 %n) nounwind { call void @llvm.memcpy.p0i8.p0i8.i64(i8* %p, i8* %q, i64 %n, i1 false), !tbaa !2 ret void diff --git a/llvm/test/Other/cgscc-devirt-iteration.ll b/llvm/test/Other/cgscc-devirt-iteration.ll index 27892e85cec7..70f6c1f508de 100644 --- a/llvm/test/Other/cgscc-devirt-iteration.ll +++ b/llvm/test/Other/cgscc-devirt-iteration.ll @@ -112,7 +112,7 @@ define void @test3(i8* %src, i8* %dest, i64 %size) noinline { ; CHECK-NOT: read ; CHECK-SAME: noinline ; BEFORE-LABEL: define void @test3(i8* %src, i8* %dest, i64 %size) -; AFTER-LABEL: define void @test3(i8* nocapture readonly %src, i8* nocapture %dest, i64 %size) +; AFTER-LABEL: define void @test3(i8* nocapture readonly %src, i8* nocapture writeonly %dest, i64 %size) %fptr = alloca i8* (i8*, i8*, i64)* store i8* (i8*, i8*, i64)* @memcpy, i8* (i8*, i8*, i64)** %fptr %f = load i8* (i8*, i8*, i64)*, i8* (i8*, i8*, i64)** %fptr diff --git a/llvm/test/Transforms/FunctionAttrs/norecurse.ll b/llvm/test/Transforms/FunctionAttrs/norecurse.ll index f5af6406e2a4..af63da176d5c 100644 --- a/llvm/test/Transforms/FunctionAttrs/norecurse.ll +++ b/llvm/test/Transforms/FunctionAttrs/norecurse.ll @@ -50,7 +50,7 @@ declare i32 @k() readnone ; CHECK: Function Attrs ; CHECK-SAME: nounwind ; CHECK-NOT: norecurse -; CHECK-NEXT: define void @intrinsic(i8* nocapture %dest, i8* nocapture readonly %src, i32 %len) +; CHECK-NEXT: define void @intrinsic(i8* nocapture writeonly %dest, i8* nocapture readonly %src, i32 %len) define void @intrinsic(i8* %dest, i8* %src, i32 %len) { call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 %len, i1 false) ret void diff --git a/llvm/test/Transforms/FunctionAttrs/writeonly.ll b/llvm/test/Transforms/FunctionAttrs/writeonly.ll index 54d00d355f7a..5094d6914929 100644 --- a/llvm/test/Transforms/FunctionAttrs/writeonly.ll +++ b/llvm/test/Transforms/FunctionAttrs/writeonly.ll @@ -78,15 +78,23 @@ define void @direct1(i8* %p) { declare void @direct2_callee(i8* %p) writeonly +; writeonly w/o nocapture is not enough ; CHECK: define void @direct2(i8* %p) define void @direct2(i8* %p) { call void @direct2_callee(i8* %p) + ; read back from global, read through pointer... ret void } -declare void @direct3_callee(i8* writeonly %p) +; CHECK: define void @direct2b(i8* nocapture writeonly %p) +define void @direct2b(i8* %p) { + call void @direct2_callee(i8* nocapture %p) + ret void +} -; CHECK: define void @direct3(i8* %p) +declare void @direct3_callee(i8* nocapture writeonly %p) + +; CHECK: define void @direct3(i8* nocapture writeonly %p) define void @direct3(i8* %p) { call void @direct3_callee(i8* %p) ret void @@ -98,15 +106,15 @@ define void @fptr_test1(i8* %p, void (i8*)* %f) { ret void } -; CHECK: define void @fptr_test2(i8* %p, void (i8*)* nocapture readonly %f) +; CHECK: define void @fptr_test2(i8* nocapture writeonly %p, void (i8*)* nocapture readonly %f) define void @fptr_test2(i8* %p, void (i8*)* %f) { - call void %f(i8* writeonly %p) + call void %f(i8* nocapture writeonly %p) ret void } -; CHECK: define void @fptr_test3(i8* %p, void (i8*)* nocapture readonly %f) +; CHECK: define void @fptr_test3(i8* nocapture writeonly %p, void (i8*)* nocapture readonly %f) define void @fptr_test3(i8* %p, void (i8*)* %f) { - call void %f(i8* %p) writeonly + call void %f(i8* nocapture %p) writeonly ret void }