[PATCH] Move machine type auto-detection to SymbolTable.

Peter Collingbourne peter at pcc.me.uk
Fri May 29 14:51:35 PDT 2015


On Fri, May 29, 2015 at 01:53:20PM -0700, Rui Ueyama wrote:
> On Fri, May 29, 2015 at 1:49 PM, Peter Collingbourne <peter at pcc.me.uk>
> wrote:
> 
> > ================
> > Comment at: COFF/SymbolTable.cpp:174-184
> > @@ -173,2 +173,13 @@
> >
> > +llvm::COFF::MachineTypes SymbolTable::getInferredMachineType() {
> > +  for (std::unique_ptr<ObjectFile> &File : ObjectFiles) {
> > +    // Try to infer machine type from the magic byte of the object file.
> > +    auto MT =
> > +
> > static_cast<llvm::COFF::MachineTypes>(File->getCOFFObj()->getMachine());
> > +    if (MT != llvm::COFF::IMAGE_FILE_MACHINE_UNKNOWN)
> > +      return MT;
> > +  }
> > +  return llvm::COFF::IMAGE_FILE_MACHINE_UNKNOWN;
> > +}
> > +
> >  } // namespace coff
> > ----------------
> > ruiu wrote:
> > > I'd make this a non-member function in Writer.cpp which takes
> > vector<unique_ptr<ObjectFile>>& as a parameter, as this function is not
> > that related to symbol resolution. It feels that it belongs Writer. Maybe
> > you need to expose ObjectFiles, but that's fine.
> > >
> > > And in the Writer, you can return Config->MachineType as a default value
> > instead of UNKNOWN.
> > `Config->MachineType` overrides the detected value, doesn't it? That's
> > what the existing logic seems to implement.
> 
> 
> Ah, yes, command line takes precedence over the detected value. Actually
> it's an error if they conflict.

Committed r238618, added a TODO to cover diagnosing conflicts.

Thanks,
-- 
Peter



More information about the llvm-commits mailing list