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

[Alp Toker]

On 04/07/2014 03:33, Peter Collingbourne wrote:
> 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)


Interesting. I'll need to read up but doesn't that approach preclude the 
lazy bitcode feature? (And keep in mind the refcounted MemoryBuffer 
patch is in review right now.)


> 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.


I've sunk it down to the C API impl in r212308. Anyway, I just realized 
you're in the middle of landing a patchset so I'll avoid refactoring 
further until you're done, sorry :-)

Alp.

>
> Thanks,

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