Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport/20.x: [LoongArch] Fix the type of tls-le symbols #133027

Merged
merged 1 commit into from
Apr 1, 2025

Conversation

zhaoqi5
Copy link
Contributor

@zhaoqi5 zhaoqi5 commented Mar 26, 2025

(cherry picked from commit d6dc74e)

@llvmbot
Copy link
Member

llvmbot commented Mar 26, 2025

@llvm/pr-subscribers-backend-loongarch

Author: ZhaoQi (zhaoqi5)

Changes

(cherry picked from commit d6dc74e)


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

2 Files Affected:

  • (modified) llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCExpr.cpp (+1)
  • (modified) llvm/test/CodeGen/LoongArch/fix-tle-le-sym-type.ll (+4-4)
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCExpr.cpp b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCExpr.cpp
index 30d2d0c1184ad..5698468c4754e 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCExpr.cpp
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCExpr.cpp
@@ -275,6 +275,7 @@ void LoongArchMCExpr::fixELFSymbolsInTLSFixups(MCAssembler &Asm) const {
   case VK_LoongArch_TLS_GD_HI20:
   case VK_LoongArch_TLS_DESC_PC_HI20:
   case VK_LoongArch_TLS_DESC_HI20:
+  case VK_LoongArch_TLS_LE_HI20_R:
   case VK_LoongArch_TLS_LD_PCREL20_S2:
   case VK_LoongArch_TLS_GD_PCREL20_S2:
   case VK_LoongArch_TLS_DESC_PCREL20_S2:
diff --git a/llvm/test/CodeGen/LoongArch/fix-tle-le-sym-type.ll b/llvm/test/CodeGen/LoongArch/fix-tle-le-sym-type.ll
index fe5f2195f0dc7..d39454a51a445 100644
--- a/llvm/test/CodeGen/LoongArch/fix-tle-le-sym-type.ll
+++ b/llvm/test/CodeGen/LoongArch/fix-tle-le-sym-type.ll
@@ -5,12 +5,12 @@
 ; RUN: llvm-readelf -s %t-la64 | FileCheck %s --check-prefix=LA64
 
 ; LA32:      Symbol table '.symtab' contains [[#]] entries:
-; LA32-NEXT:    Num:    Value  Size Type    Bind   Vis      Ndx Name
-; LA32:              00000000     0 NOTYPE  GLOBAL DEFAULT  UND tls_sym
+; LA32-NEXT:    Num:    Value  Size Type  Bind   Vis      Ndx Name
+; LA32:              00000000     0 TLS   GLOBAL DEFAULT  UND tls_sym
 
 ; LA64:      Symbol table '.symtab' contains [[#]] entries:
-; LA64-NEXT:    Num:    Value          Size Type    Bind   Vis      Ndx Name
-; LA64:              0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND tls_sym
+; LA64-NEXT:    Num:    Value          Size Type  Bind   Vis      Ndx Name
+; LA64:              0000000000000000     0 TLS   GLOBAL DEFAULT  UND tls_sym
 
 @tls_sym = external thread_local(localexec) global i32
 

Copy link
Contributor

@SixWeining SixWeining left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MaskRay
Copy link
Member

MaskRay commented Mar 26, 2025

LGTM.

Note: test/CodeGen is probably not the best place for such tests. test/MC/CSKY/relocation-specifier.s contains a nice example and tests multiple relocations at the same time.

@tstellar
Copy link
Collaborator

LGTM.

Note: test/CodeGen is probably not the best place for such tests. test/MC/CSKY/relocation-specifier.s contains a nice example and tests multiple relocations at the same time.

Will this be fixed in the main branch?

@zhaoqi5
Copy link
Contributor Author

zhaoqi5 commented Apr 1, 2025

LGTM.
Note: test/CodeGen is probably not the best place for such tests. test/MC/CSKY/relocation-specifier.s contains a nice example and tests multiple relocations at the same time.

Will this be fixed in the main branch?

Thanks. I have open a new PR (#133839) to fix this in the main branch.
Do you think we should merge this PR and then cherry-pick the new PR, or simply add the modifications of the new PR to this PR? @tstellar

@zhaoqi5
Copy link
Contributor Author

zhaoqi5 commented Apr 1, 2025

LGTM.

Note: test/CodeGen is probably not the best place for such tests. test/MC/CSKY/relocation-specifier.s contains a nice example and tests multiple relocations at the same time.

Modified in #133839, thanks.

@tstellar tstellar force-pushed the fix-tls-le-sym-type branch from 569f9be to f07f968 Compare April 1, 2025 22:58
@tstellar tstellar merged commit f07f968 into llvm:release/20.x Apr 1, 2025
8 of 12 checks passed
Copy link

github-actions bot commented Apr 1, 2025

@zhaoqi5 (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

@zhaoqi5 zhaoqi5 deleted the fix-tls-le-sym-type branch April 2, 2025 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

5 participants