summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPeter Collingbourne <peter@pcc.me.uk>2019-07-22 22:13:46 +0000
committerPeter Collingbourne <peter@pcc.me.uk>2019-07-22 22:13:46 +0000
commit710605c0853199bb447d537c3271e3e79894149e (patch)
tree33bf4e65d63a9410ab523ec6d84f7a4d71521dcf
parent95cbc3da8871f43c1ce2b2926afaedcd826202b1 (diff)
downloadbcm5719-llvm-710605c0853199bb447d537c3271e3e79894149e.tar.gz
bcm5719-llvm-710605c0853199bb447d537c3271e3e79894149e.zip
Analysis: Don't look through aliases when simplifying GEPs.
It is not safe in general to replace an alias in a GEP with its aliasee if the alias can be replaced with another definition (i.e. via strong/weak resolution (linkonce_odr) or via symbol interposition (default visibility in ELF)) while the aliasee cannot. An example of how this can go wrong is in the included test case. I was concerned that this might be a load-bearing misoptimization (it's possible for us to use aliases to share vtables between base and derived classes, and on Windows, vtable symbols will always be aliases in RTTI mode, so this change could theoretically inhibit trivial devirtualization in some cases), so I built Chromium for Linux and Windows with and without this change. The file sizes of the resulting binaries were identical, so it doesn't look like this is going to be a problem. Differential Revision: https://reviews.llvm.org/D65118 llvm-svn: 366754
-rw-r--r--llvm/lib/Analysis/ConstantFolding.cpp2
-rw-r--r--llvm/test/Analysis/ConstantFolding/gep-alias.ll17
2 files changed, 18 insertions, 1 deletions
diff --git a/llvm/lib/Analysis/ConstantFolding.cpp b/llvm/lib/Analysis/ConstantFolding.cpp
index 20231ca78b4..74f4bea41d8 100644
--- a/llvm/lib/Analysis/ConstantFolding.cpp
+++ b/llvm/lib/Analysis/ConstantFolding.cpp
@@ -784,7 +784,7 @@ Constant *CastGEPIndices(Type *SrcElemTy, ArrayRef<Constant *> Ops,
Constant* StripPtrCastKeepAS(Constant* Ptr, Type *&ElemTy) {
assert(Ptr->getType()->isPointerTy() && "Not a pointer type");
auto *OldPtrTy = cast<PointerType>(Ptr->getType());
- Ptr = Ptr->stripPointerCasts();
+ Ptr = cast<Constant>(Ptr->stripPointerCastsNoFollowAliases());
auto *NewPtrTy = cast<PointerType>(Ptr->getType());
ElemTy = NewPtrTy->getPointerElementType();
diff --git a/llvm/test/Analysis/ConstantFolding/gep-alias.ll b/llvm/test/Analysis/ConstantFolding/gep-alias.ll
new file mode 100644
index 00000000000..3be5b18fe84
--- /dev/null
+++ b/llvm/test/Analysis/ConstantFolding/gep-alias.ll
@@ -0,0 +1,17 @@
+; RUN: opt -instcombine -S -o - %s | FileCheck %s
+; Test that we don't replace an alias with its aliasee when simplifying GEPs.
+; In this test case the transformation is invalid because it replaces the
+; reference to the symbol "b" (which refers to whichever instance of "b"
+; was chosen by the linker) with a reference to "a" (which refers to the
+; specific instance of "b" in this module).
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@a = internal global [3 x i8*] zeroinitializer
+@b = linkonce_odr alias [3 x i8*], [3 x i8*]* @a
+
+define i8** @f() {
+ ; CHECK: ret i8** getelementptr ([3 x i8*], [3 x i8*]* @b, i32 0, i32 1)
+ ret i8** getelementptr ([3 x i8*], [3 x i8*]* @b, i32 0, i32 1)
+}
OpenPOWER on IntegriCloud