On 25/06/2014 08:42, David Blaikie wrote:
On Tue, Jun 24, 2014 at 10:31 PM, Alp Toker <[email protected]> wrote:
The attached patch removes code that apparently supported '.bc' binary LLVM
bitcode as a frontend input kind.

There were no tests and it's unclear if it was in a working state.
That's vaguely surprising.

I'm
comfortable removing this because it doesn't really align with the
frontend's primary objective of diagnosing textual input and lowering source
code.
It seems about as likely that someone would use this as they would use
.ll input files. I'm not sure how likely either is. As a developer I
usually use the llc, etc, tools - but sometimes I pass bitcode to
clang to link (possibly after using llvm-link to link two bitcode
files together & I want a linker invocation that has all the standard
libraries, etc rather than trying to invoke the linker myself or do
two steps of assemble+link).

It feels a little odd to push bitcode binaries through the frontend source code parsing and diagnostic paths. Perhaps the driver could handle that?

I figured the use case is to produce clang-style diagnostics that can be serialized, formatted to support different IDEs and editors etc. and that would only be relevant to '.ll'.

No strong inclination either way though if you know better.


The removal lets us parse and diagnose '.ll' IR inputs more efficiently
without duplicating file contents into memory.
Curious - how's that the case?

ParseAssembly() parses '.ll' source code and uses a LLVM SourceMgr, so has the same underlying reference counting scheme as clang's SourceManager. That means we get to not copy the whole file into memory:

-    // FIXME: This is stupid, IRReader shouldn't take ownership.
-    llvm::MemoryBuffer *MainFileCopy =
- llvm::MemoryBuffer::getMemBufferCopy(MainFile->getBuffer(),
-                                           getCurrentFile());
-
     llvm::SMDiagnostic Err;
-    TheModule.reset(ParseIR(MainFileCopy, Err, *VMContext));
+ TheModule.reset(ParseAssembly(const_cast<MemoryBuffer *>(MainFile), nullptr,
+                                  Err, *VMContext));

ParseIR() takes the LazyIRModule path if the input is binary, and that path steals ownership which forces us to always copy.


(If there's desire to keep '.bc' support and someone contributes tests for
it, I'm fine to explore other ways to achieve this optimisation.)

If you want to keep the feature, I can look into spinning the memory saving another way.

Alp.



Requires the recently posted MemoryBuffer patch.

Alp.

--
http://www.nuanti.com
the browser experts


_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


--
http://www.nuanti.com
the browser experts

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to