[llvm] ad121ea - [Attributor] Manifest simplified (return) values properly

Johannes Doerfert via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 14 19:45:55 PST 2020


Author: Johannes Doerfert
Date: 2020-02-14T21:44:46-06:00
New Revision: ad121ea14d85ea3216048d4e956eb234899f41f7

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

LOG: [Attributor] Manifest simplified (return) values properly

If we simplify a function return value we have to modify the return
instructions.

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/Attributor.cpp
    llvm/test/Transforms/Attributor/IPConstantProp/PR16052.ll
    llvm/test/Transforms/Attributor/value-simplify.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 1c259694f5ee..166c9dde2c65 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -4484,11 +4484,13 @@ struct AAValueSimplifyImpl : AAValueSimplify {
     Value &QueryingValueSimplifiedUnwrapped =
         *QueryingValueSimplified.getValue();
 
-    if (isa<UndefValue>(QueryingValueSimplifiedUnwrapped))
-      return true;
-
-    if (AccumulatedSimplifiedValue.hasValue())
+    if (AccumulatedSimplifiedValue.hasValue() &&
+        !isa<UndefValue>(AccumulatedSimplifiedValue.getValue()) &&
+        !isa<UndefValue>(QueryingValueSimplifiedUnwrapped))
       return AccumulatedSimplifiedValue == QueryingValueSimplified;
+    if (AccumulatedSimplifiedValue.hasValue() &&
+        isa<UndefValue>(QueryingValueSimplifiedUnwrapped))
+      return true;
 
     LLVM_DEBUG(dbgs() << "[ValueSimplify] " << QueryingValue
                       << " is assumed to be "
@@ -4522,13 +4524,16 @@ struct AAValueSimplifyImpl : AAValueSimplify {
   ChangeStatus manifest(Attributor &A) override {
     ChangeStatus Changed = ChangeStatus::UNCHANGED;
 
-    if (!SimplifiedAssociatedValue.hasValue() ||
+    if (SimplifiedAssociatedValue.hasValue() &&
         !SimplifiedAssociatedValue.getValue())
       return Changed;
 
-    if (auto *C = dyn_cast<Constant>(SimplifiedAssociatedValue.getValue())) {
+    Value &V = getAssociatedValue();
+    auto *C = SimplifiedAssociatedValue.hasValue()
+                  ? dyn_cast<Constant>(SimplifiedAssociatedValue.getValue())
+                  : UndefValue::get(V.getType());
+    if (C) {
       // We can replace the AssociatedValue with the constant.
-      Value &V = getAssociatedValue();
       if (!V.user_empty() && &V != C && V.getType() == C->getType()) {
         LLVM_DEBUG(dbgs() << "[ValueSimplify] " << V << " -> " << *C
                           << " :: " << *this << "\n");
@@ -4638,6 +4643,44 @@ struct AAValueSimplifyReturned : AAValueSimplifyImpl {
                ? ChangeStatus::UNCHANGED
                : ChangeStatus ::CHANGED;
   }
+
+  ChangeStatus manifest(Attributor &A) override {
+    ChangeStatus Changed = ChangeStatus::UNCHANGED;
+
+    if (SimplifiedAssociatedValue.hasValue() &&
+        !SimplifiedAssociatedValue.getValue())
+      return Changed;
+
+    Value &V = getAssociatedValue();
+    auto *C = SimplifiedAssociatedValue.hasValue()
+                  ? dyn_cast<Constant>(SimplifiedAssociatedValue.getValue())
+                  : UndefValue::get(V.getType());
+    if (C) {
+      auto PredForReturned =
+          [&](Value &V, const SmallSetVector<ReturnInst *, 4> &RetInsts) {
+            // We can replace the AssociatedValue with the constant.
+            if (&V == C || V.getType() != C->getType() || isa<UndefValue>(V))
+              return true;
+            if (auto *CI = dyn_cast<CallInst>(&V))
+              if (CI->isMustTailCall())
+                return true;
+
+            for (ReturnInst *RI : RetInsts) {
+              if (RI->getFunction() != getAnchorScope())
+                continue;
+              LLVM_DEBUG(dbgs() << "[ValueSimplify] " << V << " -> " << *C
+                                << " in " << *RI << " :: " << *this << "\n");
+              if (A.changeUseAfterManifest(RI->getOperandUse(0), *C))
+                Changed = ChangeStatus::CHANGED;
+            }
+            return true;
+          };
+      A.checkForAllReturnedValuesAndReturnInsts(PredForReturned, *this);
+    }
+
+    return Changed | AAValueSimplify::manifest(A);
+  }
+
   /// See AbstractAttribute::trackStatistics()
   void trackStatistics() const override {
     STATS_DECLTRACK_FNRET_ATTR(value_simplify)
@@ -4728,7 +4771,7 @@ struct AAValueSimplifyCallSiteReturned : AAValueSimplifyReturned {
     if (auto *CI = dyn_cast<CallInst>(&getAssociatedValue()))
       if (CI->isMustTailCall())
         return ChangeStatus::UNCHANGED;
-    return AAValueSimplifyReturned::manifest(A);
+    return AAValueSimplifyImpl::manifest(A);
   }
 
   void trackStatistics() const override {

diff  --git a/llvm/test/Transforms/Attributor/IPConstantProp/PR16052.ll b/llvm/test/Transforms/Attributor/IPConstantProp/PR16052.ll
index 06ce2d579bcd..e53d4fa4306b 100644
--- a/llvm/test/Transforms/Attributor/IPConstantProp/PR16052.ll
+++ b/llvm/test/Transforms/Attributor/IPConstantProp/PR16052.ll
@@ -1,16 +1,13 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes
-; RUN: opt -S -passes=attributor -aa-pipeline='basic-aa' -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=1 < %s | FileCheck %s
+; RUN: opt -S -passes=attributor -aa-pipeline='basic-aa' -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=2 < %s | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-; FIXME: Use the undef and do not pessimise the result because of it (no !range)
 define i64 @fn2() {
 ; CHECK-LABEL: define {{[^@]+}}@fn2()
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[CONV:%.*]] = sext i32 undef to i64
-; CHECK-NEXT:    [[DIV:%.*]] = sdiv i64 8, [[CONV]]
-; CHECK-NEXT:    [[CALL2:%.*]] = call i64 @fn1(i64 [[DIV]]) #1, !range !0
+; CHECK-NEXT:    [[CALL2:%.*]] = call i64 @fn1(i64 undef) #1, !range !0
 ; CHECK-NEXT:    ret i64 [[CALL2]]
 ;
 entry:
@@ -36,6 +33,19 @@ entry:
   ret i64 %call2
 }
 
+define i64 @fn2c() {
+; CHECK-LABEL: define {{[^@]+}}@fn2c()
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[CALL2:%.*]] = call i64 @fn1(i64 42) #1, !range !0
+; CHECK-NEXT:    ret i64 [[CALL2]]
+;
+entry:
+  %conv = sext i32 undef to i64
+  %add = add i64 42, %conv
+  %call2 = call i64 @fn1(i64 %add)
+  ret i64 %call2
+}
+
 define internal i64 @fn1(i64 %p1) {
 ; CHECK-LABEL: define {{[^@]+}}@fn1
 ; CHECK-SAME: (i64 returned [[P1:%.*]])

diff  --git a/llvm/test/Transforms/Attributor/value-simplify.ll b/llvm/test/Transforms/Attributor/value-simplify.ll
index 28f0a109cd4f..265cedccd18e 100644
--- a/llvm/test/Transforms/Attributor/value-simplify.ll
+++ b/llvm/test/Transforms/Attributor/value-simplify.ll
@@ -324,4 +324,63 @@ for.end:
   ret void
 }
 
+; Check we merge undef and a constant properly.
+; FIXME fold the addition and return the constant.
+define i8 @caller0() {
+; CHECK-LABEL: define {{[^@]+}}@caller0()
+; CHECK-NEXT:    [[C:%.*]] = call i8 @callee()
+; CHECK-NEXT:    ret i8 [[C]]
+;
+  %c = call i8 @callee(i8 undef)
+  ret i8 %c
+}
+define i8 @caller1() {
+; CHECK-LABEL: define {{[^@]+}}@caller1()
+; CHECK-NEXT:    [[C:%.*]] = call i8 @callee()
+; CHECK-NEXT:    ret i8 [[C]]
+;
+  %c = call i8 @callee(i8 undef)
+  ret i8 %c
+}
+define i8 @caller2() {
+; CHECK-LABEL: define {{[^@]+}}@caller2()
+; CHECK-NEXT:    [[C:%.*]] = call i8 @callee()
+; CHECK-NEXT:    ret i8 [[C]]
+;
+  %c = call i8 @callee(i8 undef)
+  ret i8 %c
+}
+define i8 @caller_middle() {
+; CHECK-LABEL: define {{[^@]+}}@caller_middle()
+; CHECK-NEXT:    [[C:%.*]] = call i8 @callee()
+; CHECK-NEXT:    ret i8 [[C]]
+;
+  %c = call i8 @callee(i8 42)
+  ret i8 %c
+}
+define i8 @caller3() {
+; CHECK-LABEL: define {{[^@]+}}@caller3()
+; CHECK-NEXT:    [[C:%.*]] = call i8 @callee()
+; CHECK-NEXT:    ret i8 [[C]]
+;
+  %c = call i8 @callee(i8 undef)
+  ret i8 %c
+}
+define i8 @caller4() {
+; CHECK-LABEL: define {{[^@]+}}@caller4()
+; CHECK-NEXT:    [[C:%.*]] = call i8 @callee()
+; CHECK-NEXT:    ret i8 [[C]]
+;
+  %c = call i8 @callee(i8 undef)
+  ret i8 %c
+}
+define internal i8 @callee(i8 %a) {
+; CHECK-LABEL: define {{[^@]+}}@callee()
+; CHECK-NEXT:    [[C:%.*]] = add i8 42, 7
+; CHECK-NEXT:    ret i8 [[C]]
+;
+  %c = add i8 %a, 7
+  ret i8 %c
+}
+
 ; UTC_ARGS: --disable


        


More information about the llvm-commits mailing list