RFR: 8265491: Math Signum optimization for x86 [v2]

Marcus G K Williams github.com+168222+mgkwill at openjdk.java.net
Wed Apr 21 17:29:31 UTC 2021

On Wed, 21 Apr 2021 05:28:38 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Do you mean `instruct signumF_reg`? Please explain your reasoning for the proposed move.
> I am referring to moving the instruction sequence into a macro assembly routine for better code sharing, one should be enough for both float and double type, please refer to following snippet for more detail.
> https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/x86.ad#L5672

Hi @jatin-bhateja your original request was quite unclear. The highlighting of the code was incorrect and the use of 'this' made it unclear what you were suggesting and you did not explain a reason for the suggestion. 

> Can you kindly move **this** into a macro assembly routine.

Seeing "better code sharing" as the reason it's unclear to me if the proposed refactor results in any meaningful benefit for the work involved. It's also unclear if there are any hard code structure or style issues that would necessitate the move.

The refactor you suggest calls for the instruction sequences here to be moved into a src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp function that is then called here. Inside that function there is a selection between float and double but otherwise stays the same except one is formatted .AD and one is .CPP/.HPP. 

If I am doing this refactor for `instruct signumF_reg` and `instruct signumD_reg` to cut down on code duplication, I see that as close to a wash for saving of code lines and effort involved. I also think this would make the code more complex to read.

However if there are strong opinions as to why this is necessary I'd be happy to hear them.


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

More information about the hotspot-compiler-dev mailing list