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

[Philip Reames via Phabricator via llvm-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb7b308c50ae5: [funcattrs] Infer access attributes for vararg arguments (authored by reames).

Repository:
  rG LLVM Github Monorepo

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

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.395709.patch
Type: text/x-patch
Size: 3021 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211221/e2e1de86/attachment.bin>