RFR(M): 8154826: AArch64: take better advantage of base + shifted offset addressing mode

[Doerr, Martin]

Hi all,

thanks for all your comments.

we could introduce something like
x86:
bool Matcher::const_oop_prefer_decode() { return true; }
non-x86:
bool Matcher::const_oop_prefer_decode() { return Universe::narrow_oop_base() == NULL; }

all platforms:
bool Matcher::const_klass_prefer_decode() { return Universe::narrow_klass_base() == NULL; }
(Seems like matching of DecodeNKlass as operand is not implemented on x86.)

Roland, sorry that we discuss so much of my stuff in your thread for 8154826.
I think it is slightly related to your change and it touches the same files. So it might be easier to sell it as add-on to your change?

If you prefer to handle it separated from 8154826, please let me know.
I could open a new bug for it. Disadvantage would be that someone will have to take care of closed source platforms and I will need a sponsor from Oracle.

Best regards,
Martin

 
-----Original Message-----
From: Dean Long [mailto:dean.long at oracle.com] 
Sent: Mittwoch, 27. April 2016 21:18
To: Doerr, Martin <martin.doerr at sap.com>; rwestrel at redhat.com; Zoltán Majó <zoltan.majo at oracle.com>; Vladimir Kozlov <vladimir.kozlov at oracle.com>; hotspot-compiler-dev at openjdk.java.net
Subject: Re: RFR(M): 8154826: AArch64: take better advantage of base + shifted offset addressing mode

On 4/27/2016 2:11 AM, Doerr, Martin wrote:
> Hi Roland,
>
> I have removed the piece of my change which would interfere with your change.
>
> But I'd like to explain the intention of skipping the transformation on non-x86 platforms in heap-based compressed oops mode.
> (In simpler compressed oops modes decoding is very cheap so the transformation is probably good.)
> Without transformation we have: LoadConP + Storage access
> With transformation we have: LoadConN + DecodeN heap-based + Storage access
>
> I believe X86 is the only platform which can match the DecodeN heap-based into the Storage access.
>
> I guess other platforms should prefer the untransformed version:
> PPC can load the ConP from constant pool. Decoding takes a lot of instructions, because the heap base needs to get loaded.
>
> I didn't take a closer look at SPARC, but I thought it would use the constant pool as well. Not sure if the following comment is still correct:
> // On sparc loading 32-bits constant and decoding it have less
> // instructions (4) then load 64-bits constant (7).
>
>
> Therefore, I had proposed the following code to skip:
> +         // Matching decode heap based into an operand only works on X86.
> + #if !defined(X86)
> +         if ((op == Op_ConN      && Universe::narrow_oop_base()   != NULL) ||
> +             (op == Op_ConNKlass && Universe::narrow_klass_base() != NULL)) {
> +           break;
> +         }
> + #endif
> +
>
> Would this be good for aarch64 as well?
> Would you like to include code which skips the transformation in your change or should this better be discussed independently?

Martin, wouldn't your #ifdef X86 code be better as a Matcher function, 
similar to narrow_oop_use_complex_address()?

dl

> Best regards,
> Martin
>
>
> -----Original Message-----
> From: Roland Westrelin [mailto:rwestrel at redhat.com]
> Sent: Mittwoch, 27. April 2016 10:00
> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-compiler-dev at openjdk.java.net
> Subject: Re: RFR(M): 8154826: AArch64: take better advantage of base + shifted offset addressing mode
>
> Hi Martin,
>
>> seems like this issue is related to what I have sent out today:
>> RFR(S): 8154836: VM crash due to "Base pointers must match"
>> I also had to change the AddP case of final graph reshaping.
>>
>> In one part of my change, I skip the graph transformation on non-X86 platforms when we're running in heap based compressed oops mode.
>>
>> Maybe I have to remove that part of my change, or at least adapt it.
>> We should make sure that the changes don't get pushed on the same day.
> Thanks for the heads up. It looks like your change will get in before
> mine. I'll send an updated webrev once it's integrated.
>
> Roland.