[PATCH] D115964: [funcattrs] Infer access attributes for vararg arguments

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: nikic, jdoerfert, sstefan1, aeubanks, modimo.
Herald added subscribers: ormris, bollu, hiraditya, mcrosier.
reames requested review of this revision.
Herald added a project: LLVM.

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.

Depends on: D115961 <https://reviews.llvm.org/D115961>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115964

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


Index: llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll
===================================================================
--- llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll
+++ 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 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
+}
+
Index: llvm/lib/Transforms/IPO/FunctionAttrs.cpp
===================================================================
--- llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -719,16 +719,8 @@
         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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115964.395194.patch
Type: text/x-patch
Size: 3021 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211217/e993421d/attachment.bin>