[PATCH] D95518: [Debug-Info][XCOFF] support dwarf for XCOFF for assembly output

[Jason Liu via Phabricator via llvm-commits]

jasonliu added inline comments.


================
Comment at: llvm/include/llvm/MC/MCContext.h:586-590
     getXCOFFSection(StringRef Section, SectionKind K,
                     Optional<XCOFF::CsectProperties> CsectProp = None,
                     bool MultiSymbolsAllowed = false,
-                    const char *BeginSymName = nullptr);
+                    const char *BeginSymName = nullptr,
+                    unsigned SecFlags = 0);
----------------
shchenz wrote:
> hubert.reinterpretcast wrote:
> > hubert.reinterpretcast wrote:
> > > Use `Optional` with a suitable enumeration type.
> > It may make sense to add a `getXCOFFDwarfSection` function that forwards to this.
> hmm, I would let this function like this for now. There would be many redundant codes between `getXCOFFDwarfSection` and `getXCOFFSection` if we create a new `getXCOFFDwarfSection`. Let me know if you think it is profitable to add a new function. Maybe we need a big NFC patch for this change.
I think what Hubert meant is to make a call to `getXCOFFSection` inside of `getXCOFFDwarfSection` so that you don't need to pass in so many default parameters when you just want to get the Dwarf Section. i.e. The getXCOFFDwarfSection signature could be 
```
MCSectionXCOFF *
getXCOFFDwarfSection(StringRef Section, XCOFF::DwarfSectionSubtypeFlags DwarfSubtypeFlags, bool MultiSymbolsAllowed = true, const char *BeginSymName = nullptr);
```


================
Comment at: llvm/lib/MC/MCContext.cpp:687-688
+      IsDWARFSec
+          ? XCOFFSectionKey{Section.str(), DwarfSectionSubtypeFlags.getValue()}
+          : XCOFFSectionKey{Section.str(), CsectProp->MappingClass},
+      nullptr));
----------------
Use `(``)` to make it obvious that we are calling a constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95518