[PATCH] D72347: [NFC][XCOFF] Refactor Csect creation into TargetLoweringObjectFile
David Tenty via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 15 10:31:32 PST 2020
daltenty marked an inline comment as done.
daltenty added inline comments.
================
Comment at: llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h:251
+ MCSection *
+ getSectionForExternalReference(const GlobalObject *GO,
+ const TargetMachine &TM) const override;
----------------
jasonliu wrote:
> daltenty wrote:
> > DiggerLin wrote:
> > > I think we can
> > > getSectionForExternalReference(const MCSymbol *Sym). const override here ?
> > >
> > > I search all the place where invoke getSectionForExternalReference , it can use the function define getSectionForExternalReference(const MCSymbol *Sym)
> > >
> > > the benefit of using getSectionForExternalReference(const MCSymbol *Sym) , we save one parameter and have some function parameter as getSectionForFunctionDescriptor and getSectionForTOCEntry
> > I was original inclined to do this as well, but we need to query the GlobalObject to determine whether it is a function, so we can set the correct Storage Mapping Class.
> Have we thought about passing an extra bool parameter to indicate whether it is a function or not? I don't like bool parameter either, but in this case, it might not be that bad, and we could avoid querying if we need to create a new name.
Unfortunately functions are not the only special case of externals we will need to handle in the future (e.g. thread locals), so I think this interface creates a better abstraction for the future. The current parameter types are also fairly congruent with what we see for some of the other get*Section() members here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72347/new/
https://reviews.llvm.org/D72347
More information about the llvm-commits
mailing list