RFR(s): 8187547: PPC64: icache invalidation is incorrect in some places

[Doerr, Martin]

Hi Anton,

thank you very much for providing a fix. Looks correct.

In your current version, other_insn_offset is always negative.
I'd prefer to make it always positive and simplify the usage like:
assert(other_insn_offset > 0, "first instruction must be found");
start = addr - other_insn_offset;
range = BytesPerInstWord + other_insn_offset;
This would be better readable. Would you agree?

At the moment, jdk10 repos are temporarily closed, but we can sponsor the change when it's open again and after a 2nd review.
Backports will also need to get addressed.

Please note that ppc-aix-port-dev is not appropriate for reviews because the PPC64 port is part of the main repos. Therefore, I've added hotspot-runtime-dev.

Best regards,
Martin


-----Original Message-----
From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-bounces at openjdk.java.net] On Behalf Of Anton Kozlov
Sent: Donnerstag, 14. September 2017 15:06
To: ppc-aix-port-dev at openjdk.java.net
Subject: RFR(s): 8187547: PPC64: icache invalidation is incorrect in some places

Hi, All!

Icache invalidation range calculation in NativeMovConstReg::set_data_plain and NativeMovConstReg::set_narrow_oop is incorrect and could cause VM crash:

https://bugs.openjdk.java.net/browse/JDK-8187547

I suppose the root is in mismatch of supposed and actual return values of MacroAssembler::patch_set_narrow_oop and MacroAssembler::patch_calculate_address_from_global_toc_at.
These functions takes address of the middle of sequence and expected to return first instruction offset (negative by current implementation). Instead of this, they return `-offset == abs(offset)` and offset to `data` respectively.

Supposed fix: http://cr.openjdk.java.net/~akozlov/8187547/webrev.01/

Thanks,
Anton