[PATCH] D27077: LTO: Port the new LTO API to ModuleSymbolTable.
Mehdi AMINI via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 30 20:39:11 PST 2016
mehdi_amini added inline comments.
================
Comment at: llvm/include/llvm/LTO/LTO.h:104
friend LTO;
- InputFile() = default;
+ InputFile(MemoryBufferRef MBRef) : MBRef(MBRef) {}
----------------
pcc wrote:
> mehdi_amini wrote:
> > Why don't you take all the members here?
> > It seems strange to see the initialization and then `File->Mod = ...`
> We need to create the module using the LLVMContext owned by this class.
>
> I suppose that given that we need to do that we might as well be consistent with the other members and let MBRef be default initialized as well.
OK, I missed this! But indeed I feel it is less confusing now.
================
Comment at: llvm/include/llvm/LTO/LTO.h:171
StringRef getName() const { return Name; }
StringRef getIRName() const {
+ if (isGV())
----------------
pcc wrote:
> mehdi_amini wrote:
> > Non relevant for this patch, but it'd be nice to document getName vs getIRName, I always forget when I look at this.
> It looks like getIRName is only used by the implementation, so I've removed it in r288302 and added documentation for getName.
It seems you need to rebase this patch?
https://reviews.llvm.org/D27077
More information about the llvm-commits
mailing list