[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