Re: [PATCH 1/3] [ARC] Add arithmetic and logic instructions for nps

[Graham Markall <graham dot markall at embecosm dot com>]

Thanks for showing me these details - I'm afraid I still don't
understand why the proposed functions are the same as the default
behaviour though. If you could explain any misunderstandings I have in
the following, I'd be grateful:

> In include/opcode/arc.h for arc_operand we can find:
> 
>      If it is NULL, execute
> 	 i |= (op & ((1 << o->bits) - 1)) << o->shift;
> 
> which looks identical like your proposed insert function.

The proposed insert function wraps the top end of the range back to 0
before inserting:

  if (value == UPPER)
    value = 0;

Whilst I expect this would also happen with the default implementation,
to get 8 to be accepted for a 3 bit operand I think I'd have to set
ARC_OPERAND_NCHK, but then I would no longer be checking for the exact
range 1-8 of the operand (e.g. 9 would be accepted and get inserted as
1, etc.).

> And for extract we have:
> 
>      If it is NULL, compute
> 	 op = ((i) >> o->shift) & ((1 << o->bits) - 1);
> 	 if ((o->flags & ARC_OPERAND_SIGNED) != 0
> 	     && (op & (1 << (o->bits - 1))) != 0)
> 	   op -= 1 << o->bits;
> 
> which looks like your extract function as described by MAKE_1BASED_INSERT_EXTRACT_FUNCS macro.

The proposed extract function also undoes the wraparound of the top
value in the range. I don't think the default behaviour would undo the
wraparound, because the operands that use the proposed insert functions
are unsigned.

> The range should be checked also based on the number of bits and signees of the operand. 

Is this the default insert behaviour, or a general guideline when
writing insert functions? Either way, I'm a little bit confused - if the
default insert behaviour performs a check based on the number of bits,
how could I have an operand with range 1-8 that fits into 3 bits (my
expectation is that the representation of 8 would under normal
circumstances require 4 bits, and would fail the check)?


Many thanks,
Graham.

Attachment: signature.asc
Description: OpenPGP digital signature