[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)

2024-05-17 Thread Tom Stellard via llvm-branch-commits 

https://github.com/tstellar closed 
https://github.com/llvm/llvm-project/pull/92468
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)

2024-05-17 Thread Tom Stellard via llvm-branch-commits 

https://github.com/tstellar updated 
https://github.com/llvm/llvm-project/pull/92468

>From 3d0752b9492efd60e85aedec79676596af6fb4f8 Mon Sep 17 00:00:00 2001
From: DianQK 
Date: Fri, 17 May 2024 05:51:49 +0800
Subject: [PATCH] [GlobalOpt] Don't replace aliasee with alias that has weak
 linkage (#91483)

Fixes #91312.

Don't perform the transform if the alias may be replaced at link time.

(cherry picked from commit c79690040acf5bb3d857558b0878db47f7f23dc3)
---
 llvm/lib/Transforms/IPO/GlobalOpt.cpp|  3 ++
 llvm/test/Transforms/GlobalOpt/alias-weak.ll | 57 
 2 files changed, 60 insertions(+)
 create mode 100644 llvm/test/Transforms/GlobalOpt/alias-weak.ll

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 , const 
LLVMUsed ) {
 
 static bool hasUsesToReplace(GlobalAlias , const LLVMUsed ,
  bool ) {
+  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 0..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
+;

___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)

2024-05-17 Thread via llvm-branch-commits 

DianQK wrote:

> > Per [#91483 
> > (comment)](https://github.com/llvm/llvm-project/pull/91483#issuecomment-2116394616),
> >  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 
https://github.com/llvm/llvm-project/issues/91312#issuecomment-2116404306.

https://github.com/llvm/llvm-project/pull/92468
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)

2024-05-17 Thread Tom Stellard via llvm-branch-commits 

tstellar wrote:

> Per [#91483 
> (comment)](https://github.com/llvm/llvm-project/pull/91483#issuecomment-2116394616),
>  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?

https://github.com/llvm/llvm-project/pull/92468
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)

2024-05-16 Thread via llvm-branch-commits 

DianQK wrote:

Per https://github.com/llvm/llvm-project/pull/91483#issuecomment-2116394616, we 
still need to further investigate this issue, but it won't stop us from 
backporting it.

cc @MaskRay 

https://github.com/llvm/llvm-project/pull/92468
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)

2024-05-16 Thread via llvm-branch-commits 

DianQK wrote:

Release note (maybe):
Do not replace an aliasee with an alias that has weak linkage, as this prevents 
the selection of incorrect symbols during the linking time.

@tstellar 

https://github.com/llvm/llvm-project/pull/92468
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)

2024-05-16 Thread via llvm-branch-commits 

DianQK wrote:

> 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 `Ret` variable.


https://github.com/llvm/llvm-project/pull/92468
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)

2024-05-16 Thread Eli Friedman via llvm-branch-commits 

https://github.com/efriedma-quic approved this pull request.

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.

https://github.com/llvm/llvm-project/pull/92468
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)

2024-05-16 Thread via llvm-branch-commits 

llvmbot wrote:




@llvm/pr-subscribers-llvm-transforms

Author: None (llvmbot)


Changes

Backport c79690040acf5bb3d857558b0878db47f7f23dc3

Requested by: @DianQK

---
Full diff: https://github.com/llvm/llvm-project/pull/92468.diff


2 Files Affected:

- (modified) llvm/lib/Transforms/IPO/GlobalOpt.cpp (+3) 
- (added) llvm/test/Transforms/GlobalOpt/alias-weak.ll (+57) 


``diff
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 , const 
LLVMUsed ) {
 
 static bool hasUsesToReplace(GlobalAlias , const LLVMUsed ,
  bool ) {
+  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 0..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
+;

``




https://github.com/llvm/llvm-project/pull/92468
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)

2024-05-16 Thread via llvm-branch-commits 

llvmbot wrote:

@efriedma-quic What do you think about merging this PR to the release branch?

https://github.com/llvm/llvm-project/pull/92468
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)

2024-05-16 Thread via llvm-branch-commits 

https://github.com/llvmbot milestoned 
https://github.com/llvm/llvm-project/pull/92468
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)

2024-05-16 Thread via llvm-branch-commits 

https://github.com/llvmbot created 
https://github.com/llvm/llvm-project/pull/92468

Backport c79690040acf5bb3d857558b0878db47f7f23dc3

Requested by: @DianQK

>From 012e13b1594ba19058294d793993ad8da14c9159 Mon Sep 17 00:00:00 2001
From: DianQK 
Date: Fri, 17 May 2024 05:51:49 +0800
Subject: [PATCH] [GlobalOpt] Don't replace aliasee with alias that has weak
 linkage (#91483)

Fixes #91312.

Don't perform the transform if the alias may be replaced at link time.

(cherry picked from commit c79690040acf5bb3d857558b0878db47f7f23dc3)
---
 llvm/lib/Transforms/IPO/GlobalOpt.cpp|  3 ++
 llvm/test/Transforms/GlobalOpt/alias-weak.ll | 57 
 2 files changed, 60 insertions(+)
 create mode 100644 llvm/test/Transforms/GlobalOpt/alias-weak.ll

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 , const 
LLVMUsed ) {
 
 static bool hasUsesToReplace(GlobalAlias , const LLVMUsed ,
  bool ) {
+  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 0..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
+;

___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits