[PATCH] D115021: [funcatts] Rewrite callsite operand handling in memory access inference

[Philip Reames via Phabricator via cfe-commits]

reames abandoned this revision.
reames added a comment.

This has been replaced by a series of smaller patches implementing individual 
parts.

The last of which - parameter attributes on indirect calls - is out for review 
as D116118 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115021

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115021: [funcatts] Rewrite callsite operand handling in memory access inference

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

D115961  and D115964 
 were the next two pieces of this, both have 
now landed.

The last bit is honoring call site parameter attributes on indirect calls 
outside the SCC.  I'll post that separately and then close this review because 
everything will have been covered.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115021

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115021: [funcatts] Rewrite callsite operand handling in memory access inference

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

I'm starting to split this into individual pieces with the writeonly dependence 
removed.  The first one handles indirect calls, and is posted as D115916 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115021

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115021: [funcatts] Rewrite callsite operand handling in memory access inference

[Philip Reames via Phabricator via cfe-commits]

reames planned changes to this revision.
reames added a comment.

Needs reworked over previous patch in stack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115021

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115021: [funcatts] Rewrite callsite operand handling in memory access inference

[Philip Reames via Phabricator via cfe-commits]

reames created this revision.
reames added reviewers: jdoerfert, anna, sstefan1, aeubanks, modimo.
Herald added subscribers: ormris, bollu, hiraditya, mcrosier.
reames requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

This is a rewrite of the existing code.  It isn't even close to NFC as it fixes 
several problems with the existing code.  Changes detailed below.

1. Argument attributes on an indirect call site are honored.  We'd previously 
only been honoring these on unknown direct calls, which is just weird.
2. Consistently treat calling a function pointer as an operation which reads 
the function pointer.  Previously, we gave it different treatment based on 
which attributes were on the callsite.  (e.g. a readonly callsite vs a 
writeonly callsite changed the intepretation of the function pointer access for 
an indirect call)
3. Honor attributes on vararg function declarations when visiting a vararg 
argument.  (Previously, we aborted the entire analysis.)

In addition to the previously described functional changes, the resulting code 
is both shorter, and easier to follow.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115021

Files:
  clang/test/CodeGen/arm-cmse-attr.c
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/test/Transforms/FunctionAttrs/nocapture.ll
  llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll
  llvm/test/Transforms/FunctionAttrs/writeonly.ll

Index: llvm/test/Transforms/FunctionAttrs/writeonly.ll
===
--- llvm/test/Transforms/FunctionAttrs/writeonly.ll
+++ llvm/test/Transforms/FunctionAttrs/writeonly.ll
@@ -83,19 +83,19 @@
   ret void
 }
 
-; CHECK: define void @fptr_test1(i8* %p, void (i8*)* nocapture %f)
+; CHECK: define void @fptr_test1(i8* %p, void (i8*)* nocapture readonly %f)
 define void @fptr_test1(i8* %p, void (i8*)* %f) {
   call void %f(i8* %p)
   ret void
 }
 
-; CHECK: define void @fptr_test2(i8* %p, void (i8*)* nocapture %f)
+; CHECK: define void @fptr_test2(i8* writeonly %p, void (i8*)* nocapture readonly %f)
 define void @fptr_test2(i8* %p, void (i8*)* %f) {
   call void %f(i8* writeonly %p)
   ret void
 }
 
-; CHECK: define void @fptr_test3(i8* writeonly %p, void (i8*)* nocapture %f)
+; CHECK: define void @fptr_test3(i8* writeonly %p, void (i8*)* nocapture readonly %f)
 define void @fptr_test3(i8* %p, void (i8*)* %f) {
   call void %f(i8* %p) writeonly
   ret void
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/test/Transforms/FunctionAttrs/nocapture.ll
===
--- llvm/test/Transforms/FunctionAttrs/nocapture.ll
+++ llvm/test/Transforms/FunctionAttrs/nocapture.ll
@@ -128,7 +128,7 @@
 }
 
 
-; FNATTR: define void @nc3(void ()* nocapture %p)
+; FNATTR: define void @nc3(void ()* nocapture readonly %p)
 define void @nc3(void ()* %p) {
 	call void %p()
 	ret void
@@ -141,7 +141,7 @@
 	ret void
 }
 
-; FNATTR: define void @nc5(void (i8*)* nocapture %f, i8* nocapture