[llvm] [FunctionAttrs] Only check ArgMem effects when inferring argument attrs (PR #69571)

[Nikita Popov via llvm-commits]

https://github.com/nikic created https://github.com/llvm/llvm-project/pull/69571

When inferring readonly/writeonly on arguments, if the argument is passed to a call, we should only check the ArgMem effects implied by the call -- we don't care whether the call reads/writes non-arg memory (captured pointers are not relevant here, because they will abort the analysis entirely).

This also fixes a regression that was introduced when moving to MemoryEffects: The code was still checking the old WriteOnly attribute on functions, which no longer exists.

>From 53a85eb3b9e7f4e8215aa250fad51b13414d5ab1 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Thu, 19 Oct 2023 10:21:21 +0200
Subject: [PATCH] [FunctionAttrs] Only check ArgMem effects when inferring
 argument attrs

When inferring readonly/writeonly on arguments, if the argument is
passed to a call, we should only check the ArgMem effects implied
by the call -- we don't care whether the call reads/writes non-arg
memory (captured pointers are not relevant here, because they will
abort the analysis entirely).

This also fixes a regression that was introduced when moving to
MemoryEffects: The code was still checking the old WriteOnly
attribute on functions, which no longer exists.
---
 llvm/lib/Transforms/IPO/FunctionAttrs.cpp       |  7 ++++---
 llvm/test/Transforms/FunctionAttrs/writeonly.ll | 10 +++++-----
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index 95b3204d02beb81..c6891f72ad48d40 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -668,7 +668,8 @@ determinePointerAccessAttrs(Argument *A,
               Worklist.push_back(&UU);
       }
 
-      if (CB.doesNotAccessMemory())
+      ModRefInfo ArgMR = CB.getMemoryEffects().getModRef(IRMemLocation::ArgMem);
+      if (isNoModRef(ArgMR))
         continue;
 
       if (Function *F = CB.getCalledFunction())
@@ -683,9 +684,9 @@ determinePointerAccessAttrs(Argument *A,
       // invokes with operand bundles.
       if (CB.doesNotAccessMemory(UseIndex)) {
         /* nop */
-      } else if (CB.onlyReadsMemory() || CB.onlyReadsMemory(UseIndex)) {
+      } else if (!isModSet(ArgMR) || CB.onlyReadsMemory(UseIndex)) {
         IsRead = true;
-      } else if (CB.hasFnAttr(Attribute::WriteOnly) ||
+      } else if (!isRefSet(ArgMR) ||
                  CB.dataOperandHasImpliedAttr(UseIndex, Attribute::WriteOnly)) {
         IsWrite = true;
       } else {
diff --git a/llvm/test/Transforms/FunctionAttrs/writeonly.ll b/llvm/test/Transforms/FunctionAttrs/writeonly.ll
index 01c2139dbadf64d..8676d5284682c78 100644
--- a/llvm/test/Transforms/FunctionAttrs/writeonly.ll
+++ b/llvm/test/Transforms/FunctionAttrs/writeonly.ll
@@ -215,7 +215,7 @@ define void @direct2(ptr %p) {
 define void @direct2b(ptr %p) {
 ; FNATTRS: Function Attrs: memory(write)
 ; FNATTRS-LABEL: define {{[^@]+}}@direct2b
-; FNATTRS-SAME: (ptr nocapture [[P:%.*]]) #[[ATTR8]] {
+; FNATTRS-SAME: (ptr nocapture writeonly [[P:%.*]]) #[[ATTR8]] {
 ; FNATTRS-NEXT:    call void @direct2_callee(ptr nocapture [[P]])
 ; FNATTRS-NEXT:    ret void
 ;
@@ -304,7 +304,7 @@ define void @fptr_test2(ptr %p, ptr %f) {
 define void @fptr_test3(ptr %p, ptr %f) {
 ; FNATTRS: Function Attrs: memory(write)
 ; FNATTRS-LABEL: define {{[^@]+}}@fptr_test3
-; FNATTRS-SAME: (ptr nocapture [[P:%.*]], ptr nocapture readonly [[F:%.*]]) #[[ATTR8]] {
+; FNATTRS-SAME: (ptr nocapture writeonly [[P:%.*]], ptr nocapture readonly [[F:%.*]]) #[[ATTR8]] {
 ; FNATTRS-NEXT:    call void [[F]](ptr nocapture [[P]]) #[[ATTR8]]
 ; FNATTRS-NEXT:    ret void
 ;
@@ -320,7 +320,7 @@ define void @fptr_test3(ptr %p, ptr %f) {
 
 define void @test_argmem_none_callee(ptr %p) {
 ; FNATTRS-LABEL: define {{[^@]+}}@test_argmem_none_callee
-; FNATTRS-SAME: (ptr nocapture [[P:%.*]]) {
+; FNATTRS-SAME: (ptr nocapture readnone [[P:%.*]]) {
 ; FNATTRS-NEXT:    call void @direct1_callee(ptr nocapture [[P]]) #[[ATTR9:[0-9]+]]
 ; FNATTRS-NEXT:    ret void
 ;
@@ -335,7 +335,7 @@ define void @test_argmem_none_callee(ptr %p) {
 
 define void @test_argmem_read_callee(ptr %p) {
 ; FNATTRS-LABEL: define {{[^@]+}}@test_argmem_read_callee
-; FNATTRS-SAME: (ptr nocapture [[P:%.*]]) {
+; FNATTRS-SAME: (ptr nocapture readonly [[P:%.*]]) {
 ; FNATTRS-NEXT:    call void @direct1_callee(ptr nocapture [[P]]) #[[ATTR10:[0-9]+]]
 ; FNATTRS-NEXT:    ret void
 ;
@@ -350,7 +350,7 @@ define void @test_argmem_read_callee(ptr %p) {
 
 define void @test_argmem_write_callee(ptr %p) {
 ; FNATTRS-LABEL: define {{[^@]+}}@test_argmem_write_callee
-; FNATTRS-SAME: (ptr nocapture [[P:%.*]]) {
+; FNATTRS-SAME: (ptr nocapture writeonly [[P:%.*]]) {
 ; FNATTRS-NEXT:    call void @direct1_callee(ptr nocapture [[P]]) #[[ATTR11:[0-9]+]]
 ; FNATTRS-NEXT:    ret void
 ;