release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) #92468
Conversation
|
@efriedma-quic What do you think about merging this PR to the release branch? |
|
@llvm/pr-subscribers-llvm-transforms Author: None (llvmbot) ChangesBackport c796900 Requested by: @DianQK Full diff: https://github.com/llvm/llvm-project/pull/92468.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index 951372adcfa93..619b3f612f25f 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -2212,6 +2212,9 @@ static bool mayHaveOtherReferences(GlobalValue &GV, const LLVMUsed &U) {
static bool hasUsesToReplace(GlobalAlias &GA, const LLVMUsed &U,
bool &RenameTarget) {
+ if (GA.isWeakForLinker())
+ return false;
+
RenameTarget = false;
bool Ret = false;
if (hasUseOtherThanLLVMUsed(GA, U))
diff --git a/llvm/test/Transforms/GlobalOpt/alias-weak.ll b/llvm/test/Transforms/GlobalOpt/alias-weak.ll
new file mode 100644
index 0000000000000..aec2a56313b12
--- /dev/null
+++ b/llvm/test/Transforms/GlobalOpt/alias-weak.ll
@@ -0,0 +1,57 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --include-generated-funcs --version 4
+; RUN: opt < %s -passes=globalopt -S | FileCheck %s
+
+@f1_alias = linkonce_odr hidden alias void (), ptr @f1
+@f2_alias = linkonce_odr hidden alias void (), ptr @f2
+
+define void @foo() {
+ call void @f1_alias()
+ ret void
+}
+
+define void @bar() {
+ call void @f1()
+ ret void
+}
+
+define void @baz() {
+ call void @f2_alias()
+ ret void
+}
+
+; We cannot use `f1_alias` to replace `f1` because they are both in use
+; and `f1_alias` could be replaced at link time.
+define internal void @f1() {
+ ret void
+}
+
+; FIXME: We can use `f2_alias` to replace `f2` because `b2` is not in use.
+define internal void @f2() {
+ ret void
+}
+;.
+; CHECK: @f1_alias = linkonce_odr hidden alias void (), ptr @f1
+; CHECK: @f2_alias = linkonce_odr hidden alias void (), ptr @f2
+;.
+; CHECK-LABEL: define void @foo() local_unnamed_addr {
+; CHECK-NEXT: call void @f1_alias()
+; CHECK-NEXT: ret void
+;
+;
+; CHECK-LABEL: define void @bar() local_unnamed_addr {
+; CHECK-NEXT: call void @f1()
+; CHECK-NEXT: ret void
+;
+;
+; CHECK-LABEL: define void @baz() local_unnamed_addr {
+; CHECK-NEXT: call void @f2_alias()
+; CHECK-NEXT: ret void
+;
+;
+; CHECK-LABEL: define internal void @f1() {
+; CHECK-NEXT: ret void
+;
+;
+; CHECK-LABEL: define internal void @f2() {
+; CHECK-NEXT: ret void
+;
|
There was a problem hiding this comment.
LGTM
This only affects optimizations on weak aliases, and the logic is very simple: just don't optimize them.
As noted on the original pull request, this also affects some cases which might be safe to optimize (a weak alias where the aliasee is an internal symbol with no other references). But "optimizing" those cases doesn't really have any useful effect, anyway: it doesn't unblock any additional optimizations, and the resulting ELF is basically identical.
Yes, and it looks like only this case can be optimized. I am not considering submitting this part of the change for now. But, I will add some comments and remove this |
|
Release note (maybe): |
|
Per #91483 (comment), we still need to further investigate this issue, but it won't stop us from backporting it. cc @MaskRay |
What exactly does this mean? Was there a bug in the original patch? |
It's safe, also see #91312 (comment). |
…lvm#91483) Fixes llvm#91312. Don't perform the transform if the alias may be replaced at link time. (cherry picked from commit c796900)
Backport c796900
Requested by: @dianqk