From 0fd03feb4ba5d86835ac386eebe16cbc3cfb36ec Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd@linux.alibaba.com>
Date: Fri, 6 Aug 2021 15:10:02 +0800
Subject: [PATCH] [FuncSpec] Return changed if function is changed by
 tryToReplaceWithConstant

The may get changed before specialization by RunSCCPSolver. In other
words, the pass may change the function without specialization happens.
Add test and comment to reveal this.
And it may return No Changed if the function get changed by
RunSCCPSolver before the specialization. It looks like a potential bug.

Test Plan: check-all

Reviewed By: https://reviews.llvm.org/D107622

Differential Revision: https://reviews.llvm.org/D107622
---
 llvm/lib/Transforms/IPO/FunctionSpecialization.cpp |  6 ++-
 .../function-specialization-constant-expression.ll | 49 ++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/Transforms/FunctionSpecialization/function-specialization-constant-expression.ll

diff --git a/llvm/lib/Transforms/IPO/FunctionSpecialization.cpp b/llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
index 49d5fec..7cb1ecc 100644
--- a/llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
@@ -26,6 +26,8 @@
 // `func-specialization-max-iters`
 //   increases linearly. See discussion in https://reviews.llvm.org/D106426 for
 //   details.
+// - Don't transform the function if there is no function specialization
+// happens.
 //
 //===----------------------------------------------------------------------===//
 
@@ -760,7 +762,9 @@ bool llvm::runFunctionSpecialization(
         if (!Solver.isBlockExecutable(&BB))
           continue;
         for (auto &I : make_early_inc_range(BB))
-          FS.tryToReplaceWithConstant(&I);
+          // FIXME: The solver may make changes to the function here, so set Changed, even if later
+          // function specialization does not trigger.
+          Changed |= FS.tryToReplaceWithConstant(&I);
       }
     }
   };
diff --git a/llvm/test/Transforms/FunctionSpecialization/function-specialization-constant-expression.ll b/llvm/test/Transforms/FunctionSpecialization/function-specialization-constant-expression.ll
new file mode 100644
index 0000000..b38391d
--- /dev/null
+++ b/llvm/test/Transforms/FunctionSpecialization/function-specialization-constant-expression.ll
@@ -0,0 +1,49 @@
+; Test function specialization wouldn't crash due to constant expression.
+; Note that this test case shows that function specialization pass would
+; transform the function even if no specialization happened.
+; RUN: opt -function-specialization -S < %s | FileCheck %s
+
+; CHECK: plus:
+; CHECK-NEXT:  %{{.*}} = call i64 @func2(i64* getelementptr inbounds (%struct, %struct* @Global, i32 0, i32 3))
+; CHECK: minus:
+; CHECK-NEXT: %{{.*}} = call i64 @func2(i64* getelementptr inbounds (%struct, %struct* @Global, i32 0, i32 4))
+
+%struct = type { i8, i16, i32, i64, i64}
+@Global = internal constant %struct {i8 0, i16 1, i32 2, i64 3, i64 4}
+define internal i64 @zoo(i1 %flag) {
+entry:
+  br i1 %flag, label %plus, label %minus
+
+plus:
+  %arg = getelementptr %struct, %struct* @Global, i32 0, i32 3
+  %tmp0 = call i64 @func2(i64* %arg)
+  br label %merge
+
+minus:
+  %arg2 = getelementptr %struct, %struct* @Global, i32 0, i32 4
+  %tmp1 = call i64 @func2(i64* %arg2)
+  br label %merge
+
+merge:
+  %tmp2 = phi i64 [ %tmp0, %plus ], [ %tmp1, %minus]
+  ret i64 %tmp2
+}
+
+define internal i64 @func2(i64 *%x) {
+entry:
+  %val = ptrtoint i64* %x to i64
+  ret i64 %val
+}
+
+define internal i64 @func(i64 *%x, i64 (i64*)* %binop) {
+entry:
+  %tmp0 = call i64 %binop(i64* %x)
+  ret i64 %tmp0
+}
+
+define i64 @main() {
+    %1 = call i64 @zoo(i1 0)
+    %2 = call i64 @zoo(i1 1)
+    %3 = add i64 %1, %2
+    ret i64 %3
+}
\ No newline at end of file
-- 
2.7.4