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

[Peter Collingbourne]

On Fri, Jul 04, 2014 at 04:20:35AM +0300, Alp Toker wrote:
>
> 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?

Not necessarily. I reckon that it should be possible to support lazy bitcode
by giving the BitcodeReader ownership of an owning slice of the MemoryBuffer.
That will mean we can use MemoryBuffer in more places, but that refactoring
is for another day.

> (And keep in mind the refcounted MemoryBuffer  
> patch is in review right now.)

I can't seem to find that patch.

Thanks,
-- 
Peter