Optimization question

[Vitaly Davidovich]

On Wednesday, December 23, 2015, Krystal Mok <rednaxelafx at gmail.com> wrote:

> Comments inline below:
>
> On Wed, Dec 23, 2015 at 5:55 PM, Vitaly Davidovich <vitalyd at gmail.com
> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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.
>

Understood, thanks


>
>
>>
>>> 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.
>

No worries.

Ok so Vladimir mentioned the same thing about any access that EA doesn't
know about.  I guess I'm still unclear on why unrolling needs to happen
given array length can be deduced, loop stride is constant, and loop body
shows which arrays and indices are accessed and in what manner (read,
write, both).  It seems like all the info is there even without unrolling.
Is this just an implementation detail or am I missing something fundamental?


>
>
>>
>> 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
>>
>
>

-- 
Sent from my phone
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151223/452c4d40/attachment.html>