[llvm] b7b308c - [funcattrs] Infer access attributes for vararg arguments

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2021-12-21T09:34:14-08:00
New Revision: b7b308c50ae51629a63e759cd1171714eca7784c

URL: https://github.com/llvm/llvm-project/commit/b7b308c50ae51629a63e759cd1171714eca7784c
DIFF: https://github.com/llvm/llvm-project/commit/b7b308c50ae51629a63e759cd1171714eca7784c.diff

LOG: [funcattrs] Infer access attributes for vararg arguments

This change allows us to infer access attributes (readnone, readonly) on arguments passed to vararg functions. Since there isn't a formal argument corresponding to the parameter, they'll never be considered part of the speculative SCC, but they can still benefit from attributes on the call site or the callee function.

The main motivation here is just to simplify the code, and remove some special casing. Previously, an indirect vararg call could return more precise results than an direct vararg call which is just weird.

Differential Revision: https://reviews.llvm.org/D115964

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/FunctionAttrs.cpp
    llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index 70017cb22c2e1..516d4006894ce 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -719,16 +719,8 @@ determinePointerAccessAttrs(Argument *A,
         return Attribute::None;
       }
 
-      // Given we've explictily handled the callee operand above, what's left
-      // must be a data operand (e.g. argument or operand bundle)
-      const bool IsOperandBundleUse = UseIndex >= CB.arg_size();
-      if (UseIndex >= F->arg_size() && !IsOperandBundleUse) {
-        assert(F->isVarArg() && "More params than args in non-varargs call");
-        return Attribute::None;
-      }
-
-
-      if (CB.isArgOperand(U) && SCCNodes.count(F->getArg(UseIndex))) {
+      if (CB.isArgOperand(U) && UseIndex < F->arg_size() &&
+          SCCNodes.count(F->getArg(UseIndex))) {
         // This is an argument which is part of the speculative SCC.  Note that
         // only operands corresponding to formal arguments of the callee can
         // participate in the speculation.

diff  --git a/llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll b/llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll
index c87e591898952..cbf8543bf5748 100644
--- a/llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll
+++ b/llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll
@@ -1,14 +1,8 @@
 ; RUN: opt -function-attrs -S < %s | FileCheck %s
 ; RUN: opt -passes=function-attrs -S < %s | FileCheck %s
 
-; This checks for an iterator wraparound bug in FunctionAttrs.  The previous
-; "incorrect" behavior was inferring readonly for the %x argument in @caller.
-; Inferring readonly for %x *is* actually correct, since @va_func is marked
-; readonly, but FunctionAttrs was inferring readonly for the wrong reasons (and
-; we _need_ the readonly on @va_func to trigger the problematic code path).  It
-; is possible that in the future FunctionAttrs becomes smart enough to infer
-; readonly for %x for the right reasons, and at that point this test will have
-; to be marked invalid.
+; This checks for a previously existing iterator wraparound bug in
+; FunctionAttrs, and in the process covers corner cases with varargs.
 
 declare void @llvm.va_start(i8*)
 declare void @llvm.va_end(i8*)
@@ -24,8 +18,26 @@ define void @va_func(i32* readonly %b, ...) readonly nounwind {
 }
 
 define i32 @caller(i32* %x) {
-; CHECK-LABEL: define i32 @caller(i32* nocapture %x)
+; CHECK-LABEL: define i32 @caller(i32* nocapture readonly %x)
  entry:
   call void(i32*,...) @va_func(i32* null, i32 0, i32 0, i32 0, i32* %x)
   ret i32 42
 }
+
+define void @va_func2(i32* readonly %b, ...) {
+; CHECK-LABEL: define void @va_func2(i32* nocapture readonly %b, ...)
+ entry:
+  %valist = alloca i8
+  call void @llvm.va_start(i8* %valist)
+  call void @llvm.va_end(i8* %valist)
+  %x = call i32 @caller(i32* %b)
+  ret void
+}
+
+define i32 @caller2(i32* %x, i32* %y) {
+; CHECK-LABEL: define i32 @caller2(i32* nocapture readonly %x, i32* %y)
+ entry:
+  call void(i32*,...) @va_func2(i32* %x, i32 0, i32 0, i32 0, i32* %y)
+  ret i32 42
+}
+