[PATCH] D79864: [PowerPC] Add linker opt for PC Relative GOT indirect accesses

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCInstPrinter.cpp:110
+          printInstruction(MI, Address, O);
+          O << "\n";
+          O << SymbolName;
----------------
nemanjai wrote:
> Is there not something like `printLabel()` and `printRelocDirective()` for this? It seems odd to be manually printing it like this.
> If there isn't that's fine, just seems odd.
Yes it is a bit funny.
There is a standard way to print the label:
```
SymExpr->print(O, &MAI);
```
The problem is that if I do it that I way I get this:
```
.Lpcrel3@<<invalid>>
```
I have the `MCSymbolRefExpr::VK_PPC_PCREL_OPT` there because I need it internally but I don't want it printed.


================
Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:215
 
+    bool addLinkerOpt(MachineBasicBlock &MBB, const TargetRegisterInfo *TRI) {
+      bool MadeChange = false;
----------------
nemanjai wrote:
> The algorithm here is a bit hard to follow by just reading the code. We search forward looking for a `PLDpc`. When we find one, we search forward for a user. Once we find the user, if the user defines any registers, we search back to make sure there are no intervening uses or defs of that register.
> 
> It might be easier to follow if we do a single pass to collect all address defs and uses and then check code in between for any candidate pairs.
> Something along these lines:
> 
> ```
> struct GOTDefUsePair {
>   MachineBasicBlock::iterator DefInst;
>   MachineBasicBlock::iterator UseInst;
>   Register DefReg;
>   Register UseInstDef;
> };
> 
> SmallVector<GOTDefUsePair> Pairs;
> loop over the block
>   if (instruction is a candidate PLDpc)
>     create a pair object and push it to Pairs
>     continue
>   if (instruction is a use of any defs in Pairs)
>     if (not a kill)
>       remove pair object from Pairs (this can also be done by keeping a Valid flag for each pair)
>       continue
>     add the use iterator and register (if any) to the pair in question
> ```
> 
> Then after collecting instruction pairs, loop over the vector to eliminate invalid pairs (due to register uses/defs, etc.) and finally add the symbols.
This makes sense to me. I'll use this code.


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

https://reviews.llvm.org/D79864