[llvm] r217325 - [MCJIT] Fix a bug RuntimeDyldImpl's read/writeBytesUnaligned methods.

[David Blaikie]

On Mon, Sep 8, 2014 at 11:29 AM, Lang Hames <lhames at gmail.com> wrote:

> That's one of the intents of the RuntimeDyldChecker framework: Tests for
> any any target can be executed and verified on any host. As you surmised
> the code is never actually run, just loaded and linked, then inspected
> according to the check expressions.
>

Could you checkin a test case with a hardcoded target? (I'd suggest adding
one with a hardcoded LE too - but that's probably not too important,
everyone develops on LE, right?) Seems like it might be helpful to have
both running when developing rather than checking in & waiting for a
buildbot to cycle.

I'm not sure it's a pervasive habit, but usually if I see a buildbot
failure that I could've tested locally, I'll add a portable test to
demonstrate that.

- David


>
> - Lang.
>
> On Mon, Sep 8, 2014 at 11:23 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Mon, Sep 8, 2014 at 10:56 AM, Lang Hames <lhames at gmail.com> wrote:
>>
>>> I'm not sure I'd add a function for that, though you could use an
>>> "increment" variable to merge the loops if you like. I don't think it's
>>> really worth it though.
>>>
>>> The commit message could have been clearer, but this is certainly
>>> testable: It's is an attempt to get the RuntimeDyld regression tests
>>> working on PPC.
>>>
>>
>> Can those tests be run on other platforms with a fixed triple? (the
>> verify mode you added doesn't actually have to execute the code, right? So
>> you can run PPC tests on any platform?)
>>
>>
>>>
>>> - Lang.
>>>
>>> On Sun, Sep 7, 2014 at 9:10 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Sat, Sep 6, 2014 at 7:05 PM, Lang Hames <lhames at gmail.com> wrote:
>>>>
>>>>> Author: lhames
>>>>> Date: Sat Sep  6 21:05:26 2014
>>>>> New Revision: 217325
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=217325&view=rev
>>>>> Log:
>>>>> [MCJIT] Fix a bug RuntimeDyldImpl's read/writeBytesUnaligned methods.
>>>>>
>>>>> The previous implementation was writing to the high-bytes of integers
>>>>> on BE
>>>>> targets (when run on LE hosts).
>>>>>
>>>>
>>>> Not testable? (curious what limitations of your new test infrastructure
>>>> (or curiosities of this issue) make it not applicable to testing this bug)
>>>>
>>>> & any nice way to factor the (admittedly minor) duplication in each of
>>>> these codepaths? (could have a function that takes the increment (1 or -1),
>>>> non-type template parameter if necessary, but it looks simple/inlinable?)
>>>>
>>>>
>>>>>
>>>>> http://llvm.org/PR20640
>>>>>
>>>>>
>>>>> Modified:
>>>>>     llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
>>>>>
>>>>> Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp?rev=217325&r1=217324&r2=217325&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
>>>>> (original)
>>>>> +++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp Sat
>>>>> Sep  6 21:05:26 2014
>>>>> @@ -398,32 +398,30 @@ unsigned RuntimeDyldImpl::computeSection
>>>>>  uint64_t RuntimeDyldImpl::readBytesUnaligned(uint8_t *Src,
>>>>>                                               unsigned Size) const {
>>>>>    uint64_t Result = 0;
>>>>> -  uint8_t *Dst = reinterpret_cast<uint8_t*>(&Result);
>>>>> -
>>>>> -  if (IsTargetLittleEndian == sys::IsLittleEndianHost) {
>>>>> -    if (!sys::IsLittleEndianHost)
>>>>> -      Dst += sizeof(Result) - Size;
>>>>> -    memcpy(Dst, Src, Size);
>>>>> -  } else {
>>>>> -    Dst += Size - 1;
>>>>> -    for (unsigned i = 0; i < Size; ++i)
>>>>> -      *Dst-- = *Src++;
>>>>> -  }
>>>>> +  if (IsTargetLittleEndian) {
>>>>> +    Src += Size - 1;
>>>>> +    while (Size--)
>>>>> +      Result = (Result << 8) | *Src--;
>>>>
>>>> +  } else
>>>>> +    while (Size--)
>>>>> +      Result = (Result << 8) | *Src++;
>>>>
>>>>
>>>>>    return Result;
>>>>>  }
>>>>>
>>>>>  void RuntimeDyldImpl::writeBytesUnaligned(uint64_t Value, uint8_t
>>>>> *Dst,
>>>>>                                            unsigned Size) const {
>>>>> -  uint8_t *Src = reinterpret_cast<uint8_t*>(&Value);
>>>>> -  if (IsTargetLittleEndian == sys::IsLittleEndianHost) {
>>>>> -    if (!sys::IsLittleEndianHost)
>>>>> -      Src += sizeof(Value) - Size;
>>>>> -    memcpy(Dst, Src, Size);
>>>>> +  if (IsTargetLittleEndian) {
>>>>> +    while (Size--) {
>>>>> +      *Dst++ = Value & 0xFF;
>>>>> +      Value >>= 8;
>>>>> +    }
>>>>>    } else {
>>>>> -    Src += Size - 1;
>>>>> -    for (unsigned i = 0; i < Size; ++i)
>>>>> -      *Dst++ = *Src--;
>>>>> +    Dst += Size - 1;
>>>>> +    while (Size--) {
>>>>> +      *Dst-- = Value & 0xFF;
>>>>> +      Value >>= 8;
>>>>> +    }
>>>>>    }
>>>>>  }
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140908/7884dd20/attachment.html>