Optimization question

[Krystal Mok]

Comments inline below:

On Wed, Dec 23, 2015 at 5:55 PM, Vitaly Davidovich <vitalyd at gmail.com>
wrote:

> Hi Kris,
>
> Thanks for the reply.  Some comments below.
>
> On Wednesday, December 23, 2015, Krystal Mok <rednaxelafx at gmail.com>
> wrote:
>
>> Hi Vitaly,
>>
>> For 2), there's this bug [1] which isn't really fixed, but rather, the
>> "skip zeroing of an array fully covered by a Arrays.fill()" optimization
>> was turned off for the moment.
>>
>
> That JBS talks about ints but I take it the problem is more general and
> the opto was disabled entirely? In the case I showed above, the fill is
> direct successor of the allocation with no uses of the zero values, but of
> course if the optimization is disabled entirely that would explain it.
>

Yes, the problem is more general. There's an issue with the implementation
of the optimization, which doesn't take proper dominance information into
account, so if there's a read of an array element between the allocation
and the Arrays.fill(), it might see a non-zero value. That's not just for
int[]'s, but for all array types.
So the optimization was temporarily turned off. I haven't been following
the status of that bug, though, so I'm not sure if it's fixed in some newer
version already.


>
>> For 1), if all methods are inlined as expected, I believe it's because of
>> HotSpot C2's order of optimizations: escape analysis / scalar replacement
>> happens before loop optimizations. The latter can fully unroll your loop in
>> mean(double[] array, double[] weights) if the input array is a constant
>> like the one you tested, but it's too late -- the former cannot scalar
>> replace an array if there are stores to it with unknown indices. See [2]
>> for more details.
>>
>
> Yes everything is inlined here. So what stores are we talking about here?
> The weights array is filled, yes, but only read from in mean().  In fact,
> both arrays are read only.  Also, in this particular example the array
> indices are "known" given the actual input array is constant.
>

Oops, I misread your code (I only skimmed through it, didn't read it
carefully, sorry!)
But it's the same thing, if you have either a load or a store on a element
whose index is unknown to EA, it can't scalar replace the array.


>
> Thanks and happy holidays!
>

The same to you!

- Kris


>
>> - Kris
>>
>> [1]: https://bugs.openjdk.java.net/browse/JDK-7196857
>> [2]: ConnectionGraph::adjust_scalar_replaceable_state() in opto/escape.cpp
>>     // 3. An object is not scalar replaceable if it has a field with
>> unknown
>>     // offset (array's element is accessed in loop).
>>
>> On Wed, Dec 23, 2015 at 4:56 PM, Vitaly Davidovich <vitalyd at gmail.com>
>> wrote:
>>
>>> Hi guys,
>>>
>>> Consider code like this:
>>>
>>> static double mean(double[] array, double[] weights) {
>>>      if (array.length != weights.length) throw ...;
>>>      double sum = 0;
>>>      double wsum = 0;
>>>      for(int i = 0; i < array.length; i++) {
>>>           sum += array[i] * weights[i];
>>>           wsum += weights[i];
>>>       }
>>>       return sum / wsum;
>>> }
>>>
>>> static double mean(double[] array) {
>>>      return mean(array, allOnes(array.length));
>>> }
>>>
>>> static double[] allOnes(int n) {
>>>      double[] d = new double[n];
>>>      Arrays.fill(d, 1);
>>>      return d;
>>> }
>>>
>>> Now suppose I call mean(double[]) overload like this:
>>>
>>> double[] d = {1,2,3,4};
>>>
>>> Using 8u51 with C2 compiler:
>>>
>>> 1) it looks like the array allocation from allOnes isn't eliminated.
>>> 2) moreover it looked like array was zeroed (rep stosd with rax holding
>>> zero).  Unless I misread the asm, I thought an allocation followed by
>>> Arrays.fill skips the zeroing?
>>> 3) ideally, this case would reduce to code that just does a plain
>>> unweighted mean with no multiplication by the weight and no summation for the
>>> weighted sum (weight sum is just array length).  Is this simply too much
>>> analysis to ask for?
>>>
>>> Thanks
>>>
>>>
>>> --
>>> Sent from my phone
>>>
>>
>>
>
> --
> Sent from my phone
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151223/5e2bbcba/attachment.html>