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

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer updated this revision to Diff 372540.
SjoerdMeijer added a comment.

Thanks for commenting!
I think we want to specialise on the constant value that is passed to the value. If we specialise on the address, then I think we could get wrong results as that global may be modified. Tomorrow morning I will write some more tests to convince myself this is true. In the mean time I have regenerated the test that shows the before/after a little bit better.


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

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
===================================================================
--- llvm/test/Transforms/FunctionSpecialization/function-specialization-nonconst-glob.ll
+++ llvm/test/Transforms/FunctionSpecialization/function-specialization-nonconst-glob.ll
@@ -17,7 +17,7 @@
 ; CHECK-NEXT:    [[CALL:%.*]] = call i32 @foo.1(i32 [[X]], i32* @A)
 ; CHECK-NEXT:    br label [[RETURN:%.*]]
 ; CHECK:       if.else:
-; CHECK-NEXT:    [[CALL1:%.*]] = call i32 @foo.2(i32 [[Y:%.*]], i32* @B)
+; CHECK-NEXT:    [[CALL1:%.*]] = call i32 @foo(i32 [[Y:%.*]], i32* @B)
 ; CHECK-NEXT:    br label [[RETURN]]
 ; CHECK:       return:
 ; CHECK-NEXT:    [[RETVAL_0:%.*]] = phi i32 [ [[CALL]], [[IF_THEN]] ], [ [[CALL1]], [[IF_ELSE]] ]
@@ -60,9 +60,4 @@
 ; CHECK-NEXT:    ret i32 %add
 ; CHECK-NEXT:  }
 
-; CHECK-LABEL: define internal i32 @foo.2(i32 %x, i32* %b) {
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:    %0 = load i32, i32* @B, align 4
-; CHECK-NEXT:    %add = add nsw i32 %x, %0
-; CHECK-NEXT:    ret i32 %add
-; CHECK-NEXT:  }
+; CHECK-NOT: define internal i32 @foo.2(
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.372540.patch
Type: text/x-patch
Size: 1804 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210914/400e647b/attachment.bin>