RFR: 8267968: [PPC64] Use prefixed load and addi instructions for better performance in POWER10

Kazunori Ogata ogatak at openjdk.java.net
Sun Jun 6 20:12:04 UTC 2021


On Fri, 4 Jun 2021 06:54:05 GMT, Michihiro Horie <mhorie at openjdk.org> wrote:

>> The POWER10 processor supports prefixed load and addi instructions that have larger displacement field of up to 34-bits. We can reduce instruction cycles to load constant from TOC and load an immediate value to a register.
>> 
>> Assembler::{load|add}_const_optimized() and LoadCon[LPFD]Nodes are modified to use prefixed instructions, with fixing other functions that are affected by this change.
>> 
>> I ran jtreg test on both POWER10 and POWER8 machines by using "make test-tier1" and verified no additional fails by this change. I also ran DaCapo, Renaissance, and SPECjbb2015 on both of them and verified they run successfully.
>
> src/hotspot/cpu/ppc/ppc.ad line 2894:
> 
>> 2892:     if (loadConLNodes._small)    nodes->push(loadConLNodes._small);
>> 2893:     if (loadConLNodes._large_hi) nodes->push(loadConLNodes._large_hi);
>> 2894:     if (loadConLNodes._large_lo) nodes->push(loadConLNodes._large_lo);
> 
> Is removing the _last checking needed? lf it's needed, code related to _last should be removed such as in loadConLNodesTuple_create. Also, it would be better to use an if-else condition because it cannot happen both _small and _large_hi are non null.

loadConLNodesTuple_create initializes loadConLNodes._last points to the node that is the same as either loadConLNodes._small or loadConLNodes._larege_lo.  So load_ConLNode is added twice if we don't remove _last checking.  (I actually made this bug and spent a few days to fix it...)

The correct code here should be either: 1) use the same code as that before change, i.e., don't add _small and _large_lo, and only use _last (and _large_hi), or 2) use _small and _large_lo, and remove _last, as I modified.

I chose the option 2 to avoid confusion and to make the change symmetrical to change at [L.3459](https://github.com/openjdk/jdk/pull/4267/commits/403b789cc068ea74a0768406852bf79149b23e32#diff-d21a64a4949f298476bf91083d3b956face9a6393a08a706b071068898533082R3459), where adding _small is mandatory and _last points to other node.

If you (or other reviewers) think the option 1 is better, I can revert this change and add comments as a caveat.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4267


More information about the hotspot-dev mailing list