[PATCH] D156203: [AIX][TLS] Add backend portion for the -maix-small-local-exec-tls option.

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 24 23:20:36 PDT 2023


nemanjai added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPC.td:323-327
+                   "Produce a faster access sequence for local-exec TLS "
+                   "variables where the offset from the TLS base "
+                   "is encoded as an immediate operand (AIX 64-bit only). "
+                   "This access sequence is not used for variables larger "
+                   "than 32KB.">;
----------------
I don't think it's useful to add the entire help text here. I think something like:
```
Produce TOC-free TLS sequence for this function for 64-bit AIX
```
And describe in the comment that this is not really a Subtarget feature, but it it is used for convenience.
```
// Specifies that TLS access in any function with this target attribute should
// use the optimized TOC-free sequence (where the offset is an immediate
// off of R13 for which the linker might add fix-up code if the immediate
// is too large). Clearly, this isn't really a feature of the subtarget, but is
// used as a convenient way to affect code generation for individual functions.
```

Also, I don't think we need to use such a verbose function here. This isn't really a user-facing option, so I think we can safely abbreviate it a bit. Maybe `aix-smalltls`.


================
Comment at: llvm/lib/Target/PowerPC/PPCSubtarget.cpp:128-135
+    if (!TargetTriple.isOSAIX())
+      report_fatal_error(
+        "The aix-small-local-exec-tls attribute is only supported on AIX.\n",
+        false);
+    if (!IsPPC64)
+      report_fatal_error(
+        "The aix-small-local-exec-tls attribute is currently only supported "
----------------
I think it suffices to just fold this into a single condition:
```
if (HasAIXSmallLocalExecTLS && (!TargetTriple.isOSAIX() || !IsPPC64))
  report_fatal_error(
    "The aix-small-local-tls attribute is only supported on AIX in 64-bit mode");
```


================
Comment at: llvm/test/CodeGen/PowerPC/check-aix-small-local-exec-tls-opt.ll:8
+; RUN:   FileCheck %s --check-prefix=CHECK-NOT-SUPPORTED
+
+define dso_local signext i32 @f() {
----------------
Please add another function to this test where the attribute is specified in the IR and it is compiled without `-mattr=+aix-small-local-exec-tls` but it is being compiled for something other than 64-bit AIX.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156203/new/

https://reviews.llvm.org/D156203



More information about the llvm-commits mailing list