Re: [Qemu-devel] [PATCH 3/3] tcg/arm: improve constant loading

[andrzej zaborowski <balrogg@gmail.com>]

On 9 January 2011 23:40, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Fri, Jan 07, 2011 at 04:56:32PM +0100, andrzej zaborowski wrote:
>> On 7 January 2011 15:40, Aurelien Jarno <aurelien@aurel32.net> wrote:
>> > On Fri, Jan 07, 2011 at 01:52:25PM +0100, andrzej zaborowski wrote:
>> >> On 6 January 2011 22:54, Aurelien Jarno <aurelien@aurel32.net> wrote:
>> >> ...
>> >> >         }
>> >> > +    } else {
>> >> > +        int opc = ARITH_MOV;
>> >> > +        int rn = 0;
>> >> > +
>> >> > +        do {
>> >> > +            int i, rot;
>> >> > +
>> >> > +            i = ctz32(arg) & ~1;
>> >> > +            rot = ((32 - i) << 7) & 0xf00;
>> >> > +            tcg_out_dat_imm(s, cond, opc, rd, rn, ((arg >> i) & 0xff) | rot);
>> >> > +            arg &= ~(0xff << i);
>> >> > +
>> >> > +            opc = ARITH_ORR;
>> >> > +            rn = rd;
>> >>
>> >> I think you could get rid of rn and just use rd from the start of the
>> >> loop.  Otherwise acked by me too.
>> >>
>> >
>> > What do you mean exactly? rn has to be 0 when opc is ARITH_MOV in order
>> > to generate a correct ARM instruction.
>>
>> According to my ARM926 manual rn is ignored for MOV/MVN, perhaps it's
>> different in later revisions.
>>
>
> I have just tried, and it actually works (tried on ARMv5 and ARMv7).

Also works under qemu-arm :)

> Note that binutils is not able to disassemble such an instruction and
> outputs in qemu.log something like:
> | 0x01000008:  e3aa50ff  undefined instruction 0xe3aa50ff
>
> However what worries me the most is that the "ARM Architecture Reference
> Manual ARMv7-A and ARMv7-R edition" defines this opcode with the rn field
> as "(0)(0)(0)(0)". Looking at what it means:
>
> | An instruction is UNPREDICTABLE if:
> | [...]
> | * the pseudocode for that encoding does not indicate that a different
> |   special case applies, and a bit marked (0) or (1) in the encoding
> | diagram of an instruction is not 0 or 1 respectively.
>
> In short is it still going to work on newer CPUs?

Perhaps let's be on the safe side and use your version with rn = 0.

I think it *should* work on the new ARM ISAs because of backwards
compatibility: x works under ARMv4 & ARMv5 and x is not listed under
the differences between new and old ISA, thus it needs to work under a
new ISA.

Cheers