[PATCH} Remove '.bc' input support from the frontend

[Alp Toker]

On 25/06/2014 08:42, David Blaikie wrote:
> On Tue, Jun 24, 2014 at 10:31 PM, Alp Toker <alp at nuanti.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>

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