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

[Marcus G K Williams]

On Fri, 23 Apr 2021 08:57:48 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> 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.
>
> I think it'd be easier to understand with a signum macro in MacroAssembler. Less code duplication is good, obviously. And it's potentially useful elsewhere. Win-win-win.
> We've already factored out C2 instruction sequences for things like vabsnegd  into c2_MacroAssembler_x86.

I wave the white flag. 

Thanks for the review and suggestions @theRealAph and @jatin-bhateja.

In an effort to eradicate the 5 lines of duplication and with an eye to potential usefulness in the future I've created a twice called `C2_MacroAssembler::signum_fp` using 36 lines of code and modification of c2_MacroAssembler_x86.cpp c2_MacroAssembler_x86.hpp.

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

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