From 15cd90a2c44aacbab1fe8682b8c07101ccffb9c4 Mon Sep 17 00:00:00 2001
From: Johannes Doerfert <johannes@jdoerfert.de>
Date: Fri, 1 Nov 2019 13:42:54 -0500
Subject: [PATCH] [Attributor][FIX] Make value simplification aware of
 "complicated" attributes

We cannot simply replace arguments that carry attributes like `nest`,
`inalloca`, `sret`, and `byval`. Except for the last one, which we can
replace if it is not written, we bail for now.
---
 llvm/lib/Transforms/IPO/Attributor.cpp             | 18 +++++
 .../Transforms/FunctionAttrs/value-simplify.ll     | 84 +++++++++++++++++++++-
 2 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index ddd2ff0..9fdfed8 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -3628,8 +3628,26 @@ protected:
 struct AAValueSimplifyArgument final : AAValueSimplifyImpl {
   AAValueSimplifyArgument(const IRPosition &IRP) : AAValueSimplifyImpl(IRP) {}
 
+  void initialize(Attributor &A) override {
+    AAValueSimplifyImpl::initialize(A);
+    if (!getAssociatedFunction() || getAssociatedFunction()->isDeclaration())
+      indicatePessimisticFixpoint();
+    if (hasAttr({Attribute::InAlloca, Attribute::StructRet, Attribute::Nest},
+                /* IgnoreSubsumingPositions */ true))
+      indicatePessimisticFixpoint();
+  }
+
   /// See AbstractAttribute::updateImpl(...).
   ChangeStatus updateImpl(Attributor &A) override {
+    // Byval is only replacable if it is readonly otherwise we would write into
+    // the replaced value and not the copy that byval creates implicitly.
+    Argument *Arg = getAssociatedArgument();
+    if (Arg->hasByValAttr()) {
+      const auto &MemAA = A.getAAFor<AAMemoryBehavior>(*this, getIRPosition());
+      if (!MemAA.isAssumedReadOnly())
+        return indicatePessimisticFixpoint();
+    }
+
     bool HasValueBefore = SimplifiedAssociatedValue.hasValue();
 
     auto PredForCallSite = [&](AbstractCallSite ACS) {
diff --git a/llvm/test/Transforms/FunctionAttrs/value-simplify.ll b/llvm/test/Transforms/FunctionAttrs/value-simplify.ll
index d4bb070..92d0fbd 100644
--- a/llvm/test/Transforms/FunctionAttrs/value-simplify.ll
+++ b/llvm/test/Transforms/FunctionAttrs/value-simplify.ll
@@ -1,6 +1,10 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes
 ; RUN: opt -attributor --attributor-disable=false -attributor-annotate-decl-cs -S < %s | FileCheck %s
 ; TODO: Add max-iteration check
+
+; Disable update test checks and enable it where required.
+; UTC_ARGS: --turn off
+
 ; ModuleID = 'value-simplify.ll'
 source_filename = "value-simplify.ll"
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
@@ -192,3 +196,81 @@ define i32 @ipccp3() {
   %r = call i32 @ipccp3i(i32 7)
   ret i32 %r
 }
+
+; UTC_ARGS: --turn on
+
+; Do not touch complicated arguments (for now)
+%struct.X = type { i8* }
+define internal i32* @test_inalloca(i32* inalloca %a) {
+; CHECK-LABEL: define {{[^@]+}}@test_inalloca
+; CHECK-SAME: (i32* inalloca noalias returned writeonly [[A:%.*]])
+; CHECK-NEXT:    ret i32* [[A]]
+;
+  ret i32* %a
+}
+define i32* @complicated_args_inalloca() {
+; CHECK-LABEL: define {{[^@]+}}@complicated_args_inalloca()
+; CHECK-NEXT:    [[CALL:%.*]] = call i32* @test_inalloca(i32* noalias null)
+; CHECK-NEXT:    ret i32* [[CALL]]
+;
+  %call = call i32* @test_inalloca(i32* null)
+  ret i32* %call
+}
+
+define internal void @test_sret(%struct.X* sret %a, %struct.X** %b) {
+; CHECK-LABEL: define {{[^@]+}}@test_sret
+; CHECK-SAME: (%struct.X* sret writeonly [[A:%.*]], %struct.X** nocapture nonnull writeonly dereferenceable(8) [[B:%.*]])
+; CHECK-NEXT:    store %struct.X* [[A]], %struct.X** [[B]]
+; CHECK-NEXT:    ret void
+;
+  store %struct.X* %a, %struct.X** %b
+  ret void
+}
+define void @complicated_args_sret(%struct.X** %b) {
+; CHECK-LABEL: define {{[^@]+}}@complicated_args_sret
+; CHECK-SAME: (%struct.X** nocapture writeonly [[B:%.*]])
+; CHECK-NEXT:    call void @test_sret(%struct.X* null, %struct.X** nocapture writeonly [[B]])
+; CHECK-NEXT:    ret void
+;
+  call void @test_sret(%struct.X* null, %struct.X** %b)
+  ret void
+}
+
+define internal %struct.X* @test_nest(%struct.X* nest %a) {
+; CHECK-LABEL: define {{[^@]+}}@test_nest
+; CHECK-SAME: (%struct.X* nest noalias readnone returned [[A:%.*]])
+; CHECK-NEXT:    ret %struct.X* [[A]]
+;
+  ret %struct.X* %a
+}
+define %struct.X* @complicated_args_nest() {
+; CHECK-LABEL: define {{[^@]+}}@complicated_args_nest()
+; CHECK-NEXT:    [[CALL:%.*]] = call %struct.X* @test_nest(%struct.X* noalias null)
+; CHECK-NEXT:    ret %struct.X* [[CALL]]
+;
+  %call = call %struct.X* @test_nest(%struct.X* null)
+  ret %struct.X* %call
+}
+
+@S = external global %struct.X
+define internal void @test_byval(%struct.X* byval %a) {
+; CHECK-LABEL: define {{[^@]+}}@test_byval
+; CHECK-SAME: (%struct.X* nocapture nonnull writeonly byval align 8 dereferenceable(8) [[A:%.*]])
+; CHECK-NEXT:    [[G0:%.*]] = getelementptr [[STRUCT_X:%.*]], %struct.X* [[A]], i32 0, i32 0
+; CHECK-NEXT:    store i8* null, i8** [[G0]], align 8
+; CHECK-NEXT:    ret void
+;
+  %g0 = getelementptr %struct.X, %struct.X* %a, i32 0, i32 0
+  store i8* null, i8** %g0
+  ret void
+}
+define void @complicated_args_byval() {
+; CHECK-LABEL: define {{[^@]+}}@complicated_args_byval()
+; CHECK-NEXT:    call void @test_byval(%struct.X* nonnull align 8 dereferenceable(8) @S)
+; CHECK-NEXT:    ret void
+;
+  call void @test_byval(%struct.X* @S)
+  ret void
+}
+
+; UTC_ARGS: --turn off
-- 
2.7.4