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

[Digger via Phabricator via llvm-commits]

DiggerLin marked 4 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/include/llvm/MC/MCSectionXCOFF.h:66
   XCOFF::StorageMappingClass getMappingClass() const { return MappingClass; }
-  XCOFF::StorageClass getStorageClass() const { return StorageClass; }
+  XCOFF::StorageClass getStorageClass() const { return QualName->getStorageClass(); }
   XCOFF::SymbolType getCSectType() const { return Type; }
----------------
jasonliu wrote:
> How necessary is this convenient function? It might be better to just let people do the call to QualName themselves to avoid confusion, as storage class is not a property of MCSection any more.
what about change the function name to getQualNameSymbolStorageClass() ? I think we need a wrapper for it.


================
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.");
----------------
jasonliu wrote:
> 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. 
yes, we need to set twice as jason explain


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84765