Re: [PATCH v5 3/6] sim: or1k: add or1k target to sim

Simon Marchi
2017-10-09T13:33:34Z

On 2017-10-09 09:03 AM, Stafford Horne wrote:
>> That code looks strange to me. Enabling byte reordering (SR[LEE]) makes it so that each
>> group of 4 bytes appears swapped in memory? Is this something a program can turn on/off
>> at any time? There isn't much detail about that in the spec it seems.
>
> I have added some more comments to this in the next patch series. I am not
> to sure as to the 'why' for this functionality. But I assume its probably
> to interact with hardware like graphics cards which only support
> little-endian addressing. The SR[LEE] would only be available to change in
> supervisor mode, so not just any program could change it.
>
> The above code only handles the address translation (note for half-word
> access openrisc requires the address to be half-word aligned).

Ok, thanks for the explanation.

> FYI, also the testsuite patch aas a few long lines like:
>
> /* The sign extension produces unexpected results here. */
> SHOULD_BE_LESS_THAN_UNSIGNED_I 0xFFFFFFFF - 1, 0xFFFF /* 0xFFFF gets sign-extended to 0xFFFFFFFF. */
> SHOULD_BE_LESS_THAN_UNSIGNED_I 0xFFFF7FFF, 0x8000 /* 0x8000 gets sign-extended to 0xFFFF8000. */
>
> I guess it should be like this? I don't mind the inline comments, but I
> guess convention is convention?
>
> /* 0xFFFF gets sign-extended to 0xFFFFFFFF. */
> SHOULD_BE_LESS_THAN_UNSIGNED_I 0xFFFFFFFF - 1, 0xFFFF
> /* 0x8000 gets sign-extended to 0xFFFF8000. */
> SHOULD_BE_LESS_THAN_UNSIGNED_I 0xFFFF7FFF, 0x8000

Personally I prefer with the comment above.

Simon