[PATCH] D155600: [AIX][TLS] Produce a faster local-exec access sequence with -maix-small-local-exec-tls (And optimize when load/store offsets are 0)
Hubert Tong via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Jul 29 10:31:26 PDT 2023
hubert.reinterpretcast added inline comments.
================
Comment at: llvm/lib/Target/PowerPC/PPCMCInstLower.cpp:32-69
static MCSymbol *GetSymbolFromOperand(const MachineOperand &MO,
AsmPrinter &AP) {
const TargetMachine &TM = AP.TM;
Mangler &Mang = TM.getObjFileLowering()->getMangler();
const DataLayout &DL = AP.getDataLayout();
MCContext &Ctx = AP.OutContext;
----------------
I replaced the entire function to unconditionally use `AP.getSymbol(GV)` for `MO_GlobalAddress` cases. All LIT and test-suite cases pass in 3-stage bootstrap builds on ppc64le Linux.
This should make sense: Going through the previously-default path to get a symbol using just the name is a loss of information (which was found to be problematic for toc-data and small TLS). I see no reason to keep the lossy path.
I strongly suggest going with a generic common path (which becomes well-tested) versus having special-case paths.
Since the change is not actually NFC and this patch seems to be the thing that we know can test it, I guess it goes in with this patch.
Note: I removed the part that does things with the offset. I am not sure if belongs in this function. The name of the function, `GetSymbolFromOperand` suggests that perhaps only the symbol itself should be considered. Also, the result of this function gets passed to `GetSymbolRef`, which already expects to get the offset from the MachineOperand.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155600/new/
https://reviews.llvm.org/D155600
More information about the llvm-commits
mailing list