[PATCH] D69633: [XCOFF][AIX] Differentiate usage of label symbol and csect symbol

[Digger via Phabricator via llvm-commits]

DiggerLin accepted this revision.
DiggerLin added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/include/llvm/MC/MCSectionXCOFF.h:46
       : MCSection(SV_XCOFF, K, Begin), Name(Section), MappingClass(SMC),
-        Type(ST), StorageClass(SC) {
+        Type(ST), StorageClass(SC), QualName(QualName) {
     assert((ST == XCOFF::XTY_SD || ST == XCOFF::XTY_CM) &&
----------------
jasonliu wrote:
> DiggerLin wrote:
> > I think the name of the QualName is always same as Section Name,  I am prefer to create  the QualName inside the construct of the  MCSectionXCOFF.
> > 
> > otherwise we have to write a code which created a Qualname first. and then call the new MCSectionXCOFF(......) very time.
> Section Name here is not the same as QualName. The Section name is without mapping class.
> 
> You need to create a new MCSymbol using "getOrCreateSymbol", which is inside MCContext. I don't think it's viable to do that inside of MCSection. (MCSection is not supposed to hold a MCContext object).
yes, agree with there is not MCContext in the NCSection.


================
Comment at: llvm/lib/MC/MCContext.cpp:562
+  MCSectionXCOFF *Result = new (XCOFFAllocator.Allocate()) MCSectionXCOFF(
+      CachedName, SMC, Type, SC, Kind, cast<MCSymbolXCOFF>(QualName), Begin);
   Entry.second = Result;
----------------
jasonliu wrote:
> DiggerLin wrote:
> > the name of section is same as Qualname, if the  the Qualname is created in the MCSectionXCOFF , we do not need the code MCSymbol *QualName = getOrCreateSymbol(
> >       CachedName + "[" + XCOFF::getMappingClassString(SMC) + "]");
> > 
> > here.
> I don't think you can create that QualName inside of MCSectionXCOFF. How do you call getOrCreateSymbol inside of MCSectionXCOFF?
yes, there is not MCContext in the MCSectionXCOFF


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1768
   // Handle common symbols.
   if (GVKind.isCommon() || GVKind.isBSSLocal()) {
     unsigned Align =
----------------
DiggerLin wrote:
> what about GVKind.isBSSExtern() ? 
> I think we need to deal with as .comm output
please ignore the comment (I think we need to deal with as .comm output)    I think GVKind.isBSSExtern maybe need to put in the data section.  


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

https://reviews.llvm.org/D69633