[PATCH] D12260: CodeGen: Introduce LinkedCodeGen and teach LTOCodeGenerator to use it.
Peter Collingbourne via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 27 14:57:39 PDT 2015
On Tue, Aug 25, 2015 at 06:57:54PM -0700, Duncan P. N. Exon Smith wrote:
> > + PartFilename += utostr(I);
>
> I don't love these filenames. I know it's just for the testing tool,
> but maybe `.o.N` instead of `.oN`?
Done.
> > +++ lib/CodeGen/Parallel.cpp
>
> Should this be ParallelCodeGen.cpp or ParallelCG.cpp? I don't have a
> strong opinion myself, but I feel like that vaguely follows the file
> naming conventions better (we tend to name files a little redundantly).
Renamed to ParallelCG.cpp.
> > +#ifdef _MSC_VER
> > +// concrt.h depends on eh.h for __uncaught_exception declaration
> > +// even if we disable exceptions.
> > +#include <eh.h>
> > +#endif
>
> Can/should this go in an llvm/Support header (that also includes
> `<thread>`) somewhere?
Moved to "include/llvm/Support/thread.h" in r246218.
> (Do we already have this somewhere else? If so, maybe move it to a
> Support header in a separate commit, and then just reuse it.)
This was being used by lld; I've updated it to use the new header in r246219.
> > + raw_string_ostream OS(BC);
>
> Does `raw_svector_ostream` work here?
It does; done.
> > +std::unique_ptr<Module>
> > +LinkedCodeGen(std::unique_ptr<Module> M, ArrayRef<raw_pwrite_stream *> OSs,
>
> This name doesn't really make sense to me. Also, we use lowerCamelCase() for
> functions these days. `splitCodeGen()` maybe? (You might do better.)
Hmm, I wanted to convey that the outputs of this function are intended to be
linked together, but I can't see a good succinct way to do that. splitCodeGen()
it is then.
Thanks,
--
Peter
More information about the llvm-commits
mailing list