[10] RFR (S): C2: missing strength reduction of Math.pow(x, 2.0D) to x*x

[Vladimir Kozlov]

I agree with suggested fix. What check we have before 9 and where was it?

Why do you need to move value to tmp1?

+  movdq(tmp1, xmm1);
+  cmp64(tmp1, ExternalAddress(DOUBLE2));

Thanks,
Vladimir

On 12/12/17 11:16 AM, Vladimir Ivanov wrote:
> http://cr.openjdk.java.net/~vlivanov/8190869/webrev.00/
> https://bugs.openjdk.java.net/browse/JDK-8190869
> 
> Math.pow() was completely rewritten on 9 and important special case
> (exponent == 2.0) was missed. It caused severe performance regression
> (up to 10x) on some numeric code.
> 
> The fix is two-fold:
>    * add special case in C2 to transform pow(x,2.0) into x*x;
>    * for consistency of results between different execution modes
> (interpreter, C1, and C2), add special case into generated stub at
> MacroAssembler::fast_pow().
> 
> Some clarifications follow.
> 
> Testing:
>    * microbenchmark
>    * hs-precheckin-comp, hs-tier1, hs-tier2 (in progress)
>    * test case from 8189172 on consistency of results
> 
> The fix doesn't cover all possible cases (e.g., when exponent is not a
> constant during parsing, but becomes such later). I experimented with
> more elaborate fix, but wasn't satisfied with the results and chose
> simpler route.
> 
> I see 2 alternatives:
> 
> (1) Introduce macro node PowD and either transform it into MulD if
> exponent == 2.0D or expand it into CallLeafNode. The downside is that
> PowD should be CallNode and replacing it with a MulD requires some IR
> tweaking around it.
> 
> (2) Introduce a runtime check before calling the stub (prototype [1]).
> If exponent becomes 2.0 later, the check is folded and stub call is
> removed. The downside is that when exponent doesn't turn into 2.0, the
> check stays and duplicates the check in the stub. Unfortunately, there's
> no way to remove it from the stub, because interpreter & C1 relies on
> it. Otherwise, inconsistent computation results become possible (like
> JDK-8189172 [2]).
> 
> So, I decided to go with a simple fix and cover most common case
> (explicit usage of 2.0 with Math.pow) for now.
> 
> Best regards,
> Vladimir Ivanov
> 
> [1] http://cr.openjdk.java.net/~vlivanov/8190869/webrev.runtime_check
> [2] https://bugs.openjdk.java.net/browse/JDK-8189172