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

[Andrew Dinn]

On 17/03/15 16:11, Andrew Dinn wrote:
> 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].

Doh, apologies -- I'm afraid I am still one step behind!

The two new rules both specify an unscaled 32-bit integer register as
index and, of course, the scale value is installed as 0 in the rule. So,
the correct patch to function storeLoad is to include these two switch
cases in the set which specify a sign-extend:

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

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)