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

[PowerPC] AIX TLS allow per function configuration of initial-exec or local-dynamic model #79968

Closed
wants to merge 27 commits into from

Conversation

orcguru
Copy link
Contributor

@orcguru orcguru commented Jan 30, 2024

This is follow-up patch based on #66316 to enable two target attributes to allow configuration of TLS model at function level:

The same TLS variable can be accessed by one function using initial-exec model, and another function within the same module using local-dynamic model.

In order to allow this kind of mixture, we need to rename local-dynamic model access to the variable.

orcguru and others added 27 commits December 11, 2023 01:03
However looks like machine-scheduler is interfering, and still schedule
LoadOffsetToc ahead of the .__tls_get_mod call.
(1) Removed TLSLDAIX argument so that duplicated nodes can be eliminated.
(2) Try to reuse clobber r4 by moving TLSLDAIX ahead of LoadOffsetToc node.
(3) Add FIXME comments.
(4) Add test case to show duplicated .__tls_get_mod can be eliminated.

Below two cases are not updated yet due to environment issue. I will fix
those in next update.
Failed Tests:
  LLVM :: CodeGen/PowerPC/aix-tls-xcoff-reloc-large.ll
  LLVM :: CodeGen/PowerPC/aix-tls-xcoff-reloc.ll
The "_$TLSML" symbol did not lower through getTOCEntry().
The "_$TLSML" symbol did not lower through getTOCEntry().

The pseudo node "TLSLDAIX" now takes the _$TLSML GV node, and the
ppc-tls-dynamic-call pass is updated to fine tune the relative order
between the LoadOffset@toc node and the .__tls_get_mod node.
Since `getSectionForExternalReference` explicitly set XCOFF::XTY_SD for
the `_$TLSML` symbol reference, and that only CSType `XCOFF::XTY_ER`
symbol references are added into `UndefinedCsects`, the checks inside
`executePostLayoutBinding` are redundant.

Update test case to make sure external reference to the `_$TLSML` symbol
is not observed.
function (local-dynamic or initial-exec) on AIX.
Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff d1a83ff3e0274f26746e874d480c866bec3818d6 adb312f724f4e76a4edc0e931ffd5f3b8358ea7c -- clang/lib/Basic/Targets/PPC.cpp clang/lib/Basic/Targets/PPC.h clang/lib/Frontend/CompilerInvocation.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/test/CodeGen/PowerPC/aix-tls-model.cpp clang/test/Sema/aix-attr-tls_model.c llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h llvm/include/llvm/MC/MCExpr.h llvm/include/llvm/Target/TargetLoweringObjectFile.h llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp llvm/lib/MC/MCExpr.cpp llvm/lib/MC/XCOFFObjectWriter.cpp llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp llvm/lib/Target/PowerPC/MCTargetDesc/PPCXCOFFObjectWriter.cpp llvm/lib/Target/PowerPC/PPC.h llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp llvm/lib/Target/PowerPC/PPCISelLowering.cpp llvm/lib/Target/PowerPC/PPCISelLowering.h llvm/lib/Target/PowerPC/PPCInstrInfo.cpp llvm/lib/Target/PowerPC/PPCSubtarget.cpp llvm/lib/Target/PowerPC/PPCTLSDynamicCall.cpp
View the diff from clang-format here.
diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
index 380028fb06..bc37923d50 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -1772,11 +1772,13 @@ const char *PPCTargetLowering::getTargetNodeName(unsigned Opcode) const {
   case PPCISD::ADDIS_TLSGD_HA:  return "PPCISD::ADDIS_TLSGD_HA";
   case PPCISD::ADDI_TLSGD_L:    return "PPCISD::ADDI_TLSGD_L";
   case PPCISD::GET_TLS_ADDR:    return "PPCISD::GET_TLS_ADDR";
-  case PPCISD::GET_TLS_MOD_AIX: return "PPCISD::GET_TLS_MOD_AIX";
+  case PPCISD::GET_TLS_MOD_AIX:
+    return "PPCISD::GET_TLS_MOD_AIX";
   case PPCISD::GET_TPOINTER:    return "PPCISD::GET_TPOINTER";
   case PPCISD::ADDI_TLSGD_L_ADDR: return "PPCISD::ADDI_TLSGD_L_ADDR";
   case PPCISD::TLSGD_AIX:       return "PPCISD::TLSGD_AIX";
-  case PPCISD::TLSLD_AIX:       return "PPCISD::TLSLD_AIX";
+  case PPCISD::TLSLD_AIX:
+    return "PPCISD::TLSLD_AIX";
   case PPCISD::ADDIS_TLSLD_HA:  return "PPCISD::ADDIS_TLSLD_HA";
   case PPCISD::ADDI_TLSLD_L:    return "PPCISD::ADDI_TLSLD_L";
   case PPCISD::GET_TLSLD_ADDR:  return "PPCISD::GET_TLSLD_ADDR";

Copy link
Contributor

@amy-kwan amy-kwan left a comment

Choose a reason for hiding this comment

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

Since this patch relies on the local-dynamic patch, I think the local-dynamic changes are already in here, right? Do we know if there is a way yet to make dependent pull requests, just so reviewing this patch may be a bit easier when the local-dynamic changes are separate?

@orcguru
Copy link
Contributor Author

orcguru commented Feb 20, 2024

Since this patch relies on the local-dynamic patch, I think the local-dynamic changes are already in here, right? Do we know if there is a way yet to make dependent pull requests, just so reviewing this patch may be a bit easier when the local-dynamic changes are separate?

Oh regarding the dependency, I still didn't figure out how to create a clear dependency as before. I'm creating this second PR based on the first PR's branch, but this is still intermingled...

Since I'm working on another way to push this as heuristic into backend, I'm closing this one now.

@orcguru orcguru closed this Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants