[PATCH] D109775: [FuncSpec] Fix isConstant check for globals

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: ChuanqiXu, snehasish, jaykang10.
Herald added subscribers: ormris, hiraditya.
SjoerdMeijer requested review of this revision.
Herald added a project: LLVM.

We were incorrectly specialising (i.e. miscompiling) functions that take non-constant globals as arguments. For global variables, an `isa<Constant>(V)` checks returns true, because their address is immutable, but that doesn't mean the value is immutable.


https://reviews.llvm.org/D109775

Files:
  llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
  llvm/test/Transforms/FunctionSpecialization/function-specialization-nonconst-glob.ll


Index: llvm/test/Transforms/FunctionSpecialization/function-specialization-nonconst-glob.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/FunctionSpecialization/function-specialization-nonconst-glob.ll
@@ -0,0 +1,36 @@
+; RUN: opt -function-specialization -force-function-specialization -S < %s | FileCheck %s
+
+; Global B is not constant, so we should not specialise on B:
+;
+; CHECK:      call i32 @foo.1
+; CHECK-NOT:  @foo.2
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+
+ at A = external dso_local constant i32, align 4
+ at B = external dso_local global i32, align 4
+
+define dso_local i32 @bar(i32 %x, i32 %y) {
+entry:
+  %tobool = icmp ne i32 %x, 0
+  br i1 %tobool, label %if.then, label %if.else
+
+if.then:
+  %call = call i32 @foo(i32 %x, i32* @A)
+  br label %return
+
+if.else:
+  %call1 = call i32 @foo(i32 %y, i32* @B)
+  br label %return
+
+return:
+  %retval.0 = phi i32 [ %call, %if.then ], [ %call1, %if.else ]
+  ret i32 %retval.0
+}
+
+define internal i32 @foo(i32 %x, i32* %b) {
+entry:
+  %0 = load i32, i32* %b, align 4
+  %add = add nsw i32 %x, %0
+  ret i32 %add
+}
Index: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
===================================================================
--- llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
+++ llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
@@ -674,7 +674,7 @@
       auto *V = CS.getArgOperand(A->getArgNo());
       // TrackValueOfGlobalVariable only tracks scalar global variables.
       if (auto *GV = dyn_cast<GlobalVariable>(V))
-        if (!GV->getValueType()->isSingleValueType())
+        if (!GV->isConstant() && !GV->getValueType()->isSingleValueType())
           return false;
 
       if (isa<Constant>(V) && (Solver.getLatticeValueFor(V).isConstant() ||


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D109775.372522.patch
Type: text/x-patch
Size: 1851 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210914/040a6ee9/attachment.bin>