RFR(M): 8170991: PPC64: Bad code for initialization of short arrays

[Lindenmaier, Goetz]

HI Martin,

Looks, good, thanks!

Best regards,
  Goetz.

> -----Original Message-----
> From: Doerr, Martin
> Sent: Montag, 12. Dezember 2016 20:00
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; 'hotspot-compiler-
> dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>; Gustavo
> Serra Scalet <gustavo.scalet at eldorado.org.br>
> Subject: RE: RFR(M): 8170991: PPC64: Bad code for initialization of short arrays
> 
> Hi Götz and Gustavo,
> 
> thank you very much for reviewing and for testing.
> As using a lower value for InitArrayShortSize didn't improve Gustavo's test
> cases, I agree with Götz that this part of the change should better not be done.
> I was only able to observe shorter code in some test cases which don't seem to
> be important.
> 
> I have removed the match rule for very short arrays again and set
> InitArrayShortSize to match C1's implementation (2x unrolled loop is shorter
> with more than 9 HeapWords):
> http://cr.openjdk.java.net/~mdoerr/8170991_PPC64_ClearArray/webrev.01/
> 
> Please review.
> 
> Thanks and best regards,
> Martin
> 
> 
> -----Original Message-----
> From: Lindenmaier, Goetz
> Sent: Montag, 12. Dezember 2016 12:54
> To: Doerr, Martin <martin.doerr at sap.com>; 'hotspot-compiler-
> dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>; Gustavo
> Serra Scalet <gustavo.scalet at eldorado.org.br>
> Subject: RE: RFR(M): 8170991: PPC64: Bad code for initialization of short arrays
> 
> Hi Martin,
> 
> I had a look at your change.  I like you cleaned up the C1 implementation,
> moving the various cases to the macro assembler and reusing it.
> I'm also fine with matching for constant array sizes.
> 
> I'm not sure whether reducing InitArrayShortSize helps.
> It sure removes the simple-to-spot pattern of loading 0 too often into registers.
> This only happened if register allocation ran out of registers and
> rematerialized. And as we saw, where the 0 was rematerialized, the register
> pressure was not that high, sufficient registers to hold 0 were available :).  This
> is just an artefact of register allocation.  I would assume the processor
> internally handles this efficiently.  Loading 0 into a register should be one of the
> cheapest operations altogether!
> If you only look at the places where rematerialization happened, you miss other
> benefits in the places where there is no rematerialization necessary.
> So you optimize for the few cases with rematerialization, paying the cost in the
> many places without rematerialization.
> E.g., if 0 is used in some other places in the same code, it's there anyways.
> 
> So I'm not sure it helps with performance, but if you don't spot regressions I'm
> fine with the change to InitArrayShortSize, too.
> 
> Best regards,
>   Goetz.
> 
> 
> 
> 
> > -----Original Message-----
> > From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-
> > bounces at openjdk.java.net] On Behalf Of Doerr, Martin
> > Sent: Freitag, 9. Dezember 2016 18:01
> > To: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > dev at openjdk.java.net>; Gustavo Serra Scalet
> > <gustavo.scalet at eldorado.org.br>
> > Subject: RFR(M): 8170991: PPC64: Bad code for initialization of short
> > arrays
> >
> > Hello everybody,
> >
> >
> >
> > the new flag "InitArrayShortSize" was set to 8 on PPC64. However, this
> > leads to bad code. PPC64's C2 compiler currently does not have
> > dedicated match rules to store 0.
> >
> > Unfortunately, loading of the constant 0 got rematerialized many times
> > in some cases consuming more registers and code space than needed.
> >
> >
> >
> > An attempt to improve initialization was
> >
> > 8170094: PPC64: Keep immediate value 0 cached into a register to
> > improve performance
> >
> > but this approach has disadvantages and we had decided against it.
> >
> >
> >
> > It is possibly to implement special ClearArray nodes to improve the
> > initialization of arrays only:
> >
> > http://cr.openjdk.java.net/~mdoerr/8170991_PPC64_ClearArray/webrev.00/
> >
> >
> >
> > Please review.
> >
> >
> >
> > @Gustavo: Maybe this improves your test cases, too?
> >
> >
> >
> > Best regards,
> >
> > Martin
> >
> >