This is an archive of the discontinued LLVM Phabricator instance.

[AIX][TLS] Add target attribute for -maix-small-local-exec-tls option.
ClosedPublic

Authored by amyk on Jul 24 2023, 10:17 PM.

Details

Summary

This patch adds a target attribute for an AIX-specific option that
informs the compiler that it can use a faster access sequence for the
local-exec TLS model (formally named aix-small-local-exec-tls).

The Clang portion of this option is in D155544.
The initial implementation to generate the faster access sequence is in
D155600.

Diff Detail

Event Timeline

amyk created this revision.Jul 24 2023, 10:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 24 2023, 10:17 PM
amyk requested review of this revision.Jul 24 2023, 10:17 PM
nemanjai added inline comments.Jul 24 2023, 11:20 PM
llvm/lib/Target/PowerPC/PPC.td
323–327

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.

llvm/lib/Target/PowerPC/PPCSubtarget.cpp
128–135

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");
llvm/test/CodeGen/PowerPC/check-aix-small-local-exec-tls-opt.ll
9

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.

llvm/lib/Target/PowerPC/PPC.td
323–327

aix-smalltls is ambiguous with potential future development of the analogous local-dynamic access sequence.

llvm/lib/Target/PowerPC/PPC.td
323–327
Produce TOC-free TLS sequence for this function for 64-bit AIX

Similarly, this is only true for local-exec cases.

hubert.reinterpretcast retitled this revision from [AIX][TLS] Add backend portion for the -maix-small-local-exec-tls option. to [AIX][TLS] Add target attribute for -maix-small-local-exec-tls option..Jul 25 2023, 10:59 AM
hubert.reinterpretcast edited the summary of this revision. (Show Details)
amyk updated this revision to Diff 544035.Jul 25 2023, 11:55 AM
amyk marked 5 inline comments as done.

Address review comments:

  • Updated comments
  • Simplified conditions
  • add a new test where the attribute is in the IR
llvm/lib/Target/PowerPC/PPC.td
323–327

I've updated the comment a bit to address both Nemanja and Hubert's concerns.
As Hubert mentioned, I will probably just keep the option as aix-small-local-exec-tls.

llvm/lib/Target/PowerPC/PPCSubtarget.cpp
128–135

Yeah, I agree.
Done.

llvm/test/CodeGen/PowerPC/check-aix-small-local-exec-tls-opt-IRattribute.ll
14–15

Same comment re: check prefixes.

llvm/test/CodeGen/PowerPC/check-aix-small-local-exec-tls-opt.ll
17–18

No need for different check prefixes if the expected output is the same.

amyk updated this revision to Diff 544169.Jul 25 2023, 6:11 PM
amyk marked 2 inline comments as done.

Address Hubert's review comment of removing the duplicate CHECK.

This revision is now accepted and ready to land.Jul 25 2023, 6:39 PM
This revision was landed with ongoing or failed builds.Sep 7 2023, 6:06 PM
This revision was automatically updated to reflect the committed changes.