[clang] [PowerPC] Support local-dynamic TLS relocation on AIX (PR #66316)

Felix via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 25 23:16:36 PDT 2023


================
@@ -3412,13 +3416,23 @@ SDValue PPCTargetLowering::LowerGlobalTLSAddressAIX(SDValue Op,
     return DAG.getNode(PPCISD::ADD_TLS, dl, PtrVT, TLSReg, VariableOffset);
   }
 
-  // Only Local-Exec, Initial-Exec and General-Dynamic TLS models are currently
-  // supported models. If Local- or Initial-exec are not possible or specified,
-  // all GlobalTLSAddress nodes are lowered using the general-dynamic model.
-  // We need to generate two TOC entries, one for the variable offset, one for
-  // the region handle. The global address for the TOC entry of the region
-  // handle is created with the MO_TLSGDM_FLAG flag and the global address
-  // for the TOC entry of the variable offset is created with MO_TLSGD_FLAG.
+  if (Model == TLSModel::LocalDynamic) {
+    // For local-dynamic on AIX, we need to generate two TOC entries, one for
+    // the variable offset, the other for the module handle. The module handle
+    // is encapsulated inside the TLSLD_AIX pseudo node, and will be expanded by
+    // PPCTLSDynamicCall.
+    SDValue VariableOffsetTGA =
+        DAG.getTargetGlobalAddress(GV, dl, PtrVT, 0, PPCII::MO_TLSLD_FLAG);
+    SDValue VariableOffset = getTOCEntry(DAG, dl, VariableOffsetTGA);
----------------
orcguru wrote:

So I did have another version which can essentially remove the assert check with some side effects:

(1) It did generated extra external symbol reference, which looks like no harm with simple HelloWorld. (Notice: xlC does not generate this symbol reference.)
```
        .extern _Renamed..5f24__TLSML[UA]
        .rename _Renamed..5f24__TLSML[UA],"_$TLSML"
```
(2) Inside `XCOFFObjectWriter::recordRelocation()` logic I have to apply a hack to get the correct symbol, because now there are two symbols: one is `_Renamed..5f24__TLSML[UA]`, the other is `_Renamed..5f24__TLSML[TC]`. Without hack it will choose the [UA] one which will cause linker failure.

Attached the draft patch: [ReplaceAssertHack.diff.txt](https://github.com/llvm/llvm-project/files/12723368/ReplaceAssertHack.diff.txt)

I think current version has the advantage that it does not generate any extra external symbol reference or any extra relocation entry, although it looks ugly with regard to those asserts.

Is it possible that we fix the hack implementation with some future version (I will open an issue and work on it afterwards)? How about let's move on with current approach? 

@amy-kwan @stephenpeckham @bzEq @chenzheng1030 Appreciate your comments. Thank you!

https://github.com/llvm/llvm-project/pull/66316


More information about the cfe-commits mailing list