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

[Lang Hames]

Apologies if my previous comment was unclear, but this failure won't
reproduce on all architectures (that's one of the reasons it has taken so
long to debug). It only shows up when the host and target endianness
differ, which is why it's showing up on PPC. The aim of this patch is to
get the PPC bot green.

- Lang.


On Mon, Sep 8, 2014 at 11:37 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> 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/01810c2e/attachment.html>