[llvm] r212305 - Modify LTOModule::isTargetMatch to take a StringRef instead of a MemoryBuffer.

Peter Collingbourne peter at pcc.me.uk
Thu Jul 3 17:33:28 PDT 2014


On Fri, Jul 04, 2014 at 03:17:26AM +0300, Alp Toker wrote:
>> --- llvm/trunk/lib/LTO/LTOModule.cpp (original)
>> +++ llvm/trunk/lib/LTO/LTOModule.cpp Thu Jul  3 18:49:28 2014
>> @@ -75,7 +75,7 @@ bool LTOModule::isBitcodeFileForTarget(c
>>     MemoryBuffer *buffer = makeBuffer(mem, length);
>>     if (!buffer)
>>       return false;
>> -  return isTargetMatch(buffer, triplePrefix);
>> +  return isTargetMatch(StringRef((const char *)mem, length), triplePrefix);
>
> This creates a memory buffer, then doesn't use it.

Doh, I forgot to remove that part. Feel free to remove it if I don't
get to it first.

>
>>   }
>>     bool LTOModule::isBitcodeFileForTarget(const char *path,
>> @@ -83,15 +83,15 @@ bool LTOModule::isBitcodeFileForTarget(c
>>     std::unique_ptr<MemoryBuffer> buffer;
>>     if (MemoryBuffer::getFile(path, buffer))
>>       return false;
>> -  return isTargetMatch(buffer.release(), triplePrefix);
>> +  return isTargetMatch(buffer->getBuffer(), triplePrefix);
>
> This creates a memory buffer, then creates another in isTargetMatch()  
> when a single one would do.
>
> I've attached a nicer way to clean this up.
>
> OK to commit this over yours?

This will eventually take a StringRef in more places (see
http://reviews.llvm.org/D4371) so I would prefer to keep taking a StringRef
here.

> (Ultimately we should remove the the two wrapper functions because they  
> just encourage inefficient use by statting, mmapping and reading the  
> file twice -- once to test and then again to use it.)

I think this is baked into the tools/lto API but we can definitely do better
for gold.

Thanks,
-- 
Peter



More information about the llvm-commits mailing list