[PATCH] D84765: [AIX][XCOFF] change oprand of branch instruction from symbol name to qualify symbol name.

[Jason Liu via Phabricator via llvm-commits]

jasonliu added inline comments.


================
Comment at: llvm/include/llvm/MC/MCSectionXCOFF.h:50
     QualName->setRepresentedCsect(this);
+    QualName->setStorageClass(XCOFF::C_HIDEXT);
     // A csect is 4 byte aligned by default, except for undefined symbol csects.
----------------
Xiangling_L wrote:
> I am wondering why do we want to set a default SC here? It seems redundant.
We transfer SC property from MCSectionXCOFF to MCSymbolXCOFF. MCSectionXCOFF represents a csect. A csect by default have XCOFF::C_HIDEXT Storage class. Without this, we would not be able to get storage class from a MCSectionXCOFF when that csect do not need to `emitLinkage`.


================
Comment at: llvm/include/llvm/MC/MCSymbolXCOFF.h:38
   void setStorageClass(XCOFF::StorageClass SC) {
-    assert((!StorageClass.hasValue() || StorageClass.getValue() == SC) &&
-           "Redefining StorageClass of XCOFF MCSymbol.");
----------------
Xiangling_L wrote:
> It looks like this assertion is still useful to prevent someone from setting SC twice or setting other SC for a same symbol accidentally, if we don't need to set a default SC as I mentioned above.
If I understand correctly, we would actually set MCSectionXCOFF/csect's SC twice in some scenarios. First time being the default HIDE_EXT. Second time, when someone called emitLinkage on a csect's qualname symbol (and we could set it to C_EXT at that time. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84765