[jira] Commented: (HARMONY-2984) [drlvm][jit] More peephole optimizations implemented: IMUL and AND+CMP->TEST

["Alex Astapchuk (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HARMONY-2984?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12465647 ] 

Alex Astapchuk commented on HARMONY-2984:
-----------------------------------------

Oh yeah, good work. Thanks a lot!
All these 'MUL , 0' and 'MUL ,1' looked ridiculous (not to mention they gobble ticks).

The patch is generally ok, with some notes on details.

The way you detect whether a number is power of two is overcomplicated.
You've added two separate functions - each of them is used only once, and 
they both only used together. The meaning of 'getMinBit == getMaxBit' is 
not obvious.
I suggest to remove both of these functions and replace them with a single 
and clear function, something like 'findPowerOfTwo()'.

Also, I strongly suggest to avoid using platform-dependent code as long as possible.
This also implies 'avoid usage of asm code'. 
There is absolutely no need in using anything specific here, but all these additional 
macros and ams code adds unnecessary complexity. Don't panic about adding a single loop,
until it's in really hot code. This particular code is not hot. 
"Premature optimization is the root of all evil" (c) :-)

Also, the getting uses and defs operands looks like the very common pattern. Not only 
in your patch, but in general. I suggest to extract this code and introduce 
separate methods in OpndUtils. Something like 
	static Opnd* OpndUtils::getDef(Inst* inst, unsigned index=0)
	static Opnd* OpndUtils::getUse(Inst* inst, unsigned index)
This will simplify both this code, and further usage of xxUtils.


As the total, I suggest to commit this patch, and then fix (if agreed) the notes.





> [drlvm][jit] More peephole optimizations implemented: IMUL and AND+CMP->TEST
> ----------------------------------------------------------------------------
>
>                 Key: HARMONY-2984
>                 URL: https://issues.apache.org/jira/browse/HARMONY-2984
>             Project: Harmony
>          Issue Type: Improvement
>          Components: DRLVM
>            Reporter: Mikhail Fursov
>         Assigned To: Alexey Varlamov
>         Attachments: peephole.diff
>
>
> This patch adds 2 new peephole optimizations
> 1) IMUL 0, 1, 2 or power of 2 is simplified to ADD, MOV or SHIFT. This can not be done in HLO because these constants appears during HIR->LIR translation
> 2) AND, CMP, JZ sequence is simplified to TEST, JZ
> Linux guru, please check min/max bit asm functions in my patch. What do you think, should we use platform independed but slow algorithms here instead of asm?

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira