[9] RFR(S): 8075136: Unnecessary sign extension for byte array access

[Andrew Dinn]

On 17/03/15 15:19, Andrew Dinn wrote:
> On 17/03/15 15:03, Roland Westrelin wrote:
>>
>>> http://cr.openjdk.java.net/~thartmann/8075136/webrev.03/
>>
>> Looks good to me.
> 
> I'm not officially a JDK9 reviewer but . . .
> 
> I believe this is a mistake. See my follow up in another thread.

Apologies for jumping the gun wiht my previous comment. Having reviewed
the earlier traffic in this thread I now understand why the two new
rules are needed (i.e. to deal with the case where there is no LShift in
the matched operand). Since they require use of an offset and an
(unscaled) index register both these new operands will need to be
encoded using two instructions. So that has two implications:

The cost of the operand will need to be INSN_COST to account for the
extra add.

The encoding function loadStore (the first one which operates on integer
registers but not the second one which operates on float registers) will
need to be updated to recognise these two operand types and pass in a
zero scaling:

    . . .
    switch (opcode) {
    case INDINDEXSCALEDOFFSETI2L:
    case INDINDEXSCALEDI2L:
    case INDINDEXSCALEDOFFSETI2LN:
    case INDINDEXSCALEDI2LN:
      scale = Address::sxtw(size);
      break;
+   case INDINDEXOFFSETI2L:
+   case INDINDEXOFFSETI2LN:
+      scale = Address::lsl(0);
+      break;
    default:
      scale = Address::lsl(size);
    . . .

[as Andrew Haley's comment points out the need for case handling here is
horribly ugly].

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in UK and Wales under Company Registration No. 3798903
Directors: Michael Cunningham (USA), Matt Parson (USA), Charlie Peters
(USA), Michael O'Neill (Ireland)