[PATCH] D68721: [NFC][PowerPC]Clean up PPCAsmPrinter for TOC related pseudo opcode

[Xiangling Liao via Phabricator via llvm-commits]

Xiangling_L marked 2 inline comments as done.
Xiangling_L added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:774
     if (GlobalToc || MO.isJTI() || MO.isBlockAddress() ||
-        TM.getCodeModel() == CodeModel::Large)
+	(MO.isCPI() && TM.getCodeModel() == CodeModel::Large))
       MOSymbol = lookUpOrCreateTOCEntry(MOSymbol);
----------------
hubert.reinterpretcast wrote:
> Xiangling_L wrote:
> > hubert.reinterpretcast wrote:
> > > This does not seem to be an NFC change.
> > Since Machine Operand for this TOC pseudo opcode can only be `MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress()`, so the line`(MO.isCPI() && TM.getCodeModel() == CodeModel::Large)` will have the same logic with or without `MO.isCPI()`. But since people @jsji have some slight preference to make it clear, I add `MO.isCPI()`. Please let me know if you still have any concerns about this change.
> I do not believe that the statement is sufficiently supported in this context except if there is:
> ```
> assert(GlobalToc || !MO.isGlobal());
> ```
> 
I think I kinda know your concern, do you mean without my changes, for query `if (GlobalToc || MO.isJTI() || MO.isBlockAddress() || TM.getCodeModel() == CodeModel::Large)`, there is the possibility that `GlobalToc` is false, but the code model is large, under which we still generate TOCEntry for global MO? If my understanding is correct, then I think we don't need to worry about this situation, because according to the definition of `isGVIndirectSymbol`:

```
bool PPCSubtarget::isGVIndirectSymbol(const GlobalValue *GV) const {
  // Large code model always uses the TOC even for local symbols.
  if (TM.getCodeModel() == CodeModel::Large)
    return true;
  if (TM.shouldAssumeDSOLocal(*GV->getParent(), GV))
    return false;
  return true;
}

```

When it's large code model,  and `MO.isGlobal() == true`, `GlobalToc` is definitely `true`, so there is no situation that when `MO.isGlobal()` &&`GlobalToc == false`,  but the code model is not large. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68721