Webrev for first version of HSA offload integer reduce

[Christian Thalinger]

Thanks; much better.  One thing I missed during my last review is that all the tests have the wrong copyright year and an author comment:

  34 /**
  35  *
  36  * @author user1
  37  */

Can you remove these?

On Jun 10, 2014, at 8:03 AM, Caspole, Eric <Eric.Caspole at amd.com> wrote:

> I moved collectArgs into the cpp files for both classes here:
>  http://cr.openjdk.java.net/~ecaspole/graal_int_reduce1/02/webrev/
> Thanks,
> Eric
> 
> 
> From: Christian Thalinger [christian.thalinger at oracle.com]
> Sent: Monday, June 09, 2014 2:15 PM
> To: Caspole, Eric
> Cc: graal-dev at openjdk.java.net; sumatra-dev at openjdk.java.net
> Subject: Re: Webrev for first version of HSA offload integer reduce
> 
> 
> On Jun 9, 2014, at 10:50 AM, Eric Caspole <eric.caspole at amd.com> wrote:
> 
>> 
>> 
>> On 06/09/2014 01:07 PM, Christian Thalinger wrote:
>>> 
>>> On Jun 9, 2014, at 9:40 AM, Caspole, Eric <Eric.Caspole at amd.com
>>> <mailto:Eric.Caspole at amd.com>> wrote:
>>> 
>>>> Hi everybody,
>>>> I put up a new webrev that allows some IntStream.reduce() to be
>>>> offloaded to HSA:
>>>> 
>>>> http://cr.openjdk.java.net/~ecaspole/graal_int_reduce1/webrev/
>>> 
>>> 
>>>    src/gpu/hsail/vm/hsailKernelArguments.hpp
>>> 
>>> +  virtual void collectArgs() {
>>> 
>>> Does this really have to be in the .hpp file?  I see that the base class
>>> has it in the .hpp file too.
>> 
>> No it doesn't have to be there, it is just some historical thing I don't know why. I can change that.
> 
> That would be good.  Thanks.
> 
>> 
>>> 
>>>> 
>>>> This works with a Sumatra webrev I will eventually add into the
>>>> Sumatra JDK repo:
>>>> 
>>>> http://cr.openjdk.java.net/~ecaspole/sumatra_int_reduce_1/webrev/
>>>> 
>>>> This combo allows offloading IntStream.parallel().reduce() for
>>>> Integer::sum, Integer::max, and Integer::min. It builds a kernel from
>>>> a hand made HSAIL string of code that gets called from the new Stream
>>>> API diversion code in the Sumatra part.
>>>> 
>>>> Eventually we hope to have regular compilation of user reduce
>>>> functions but this allows us to do experiments with workloads before
>>>> we get all the details of reduce compilation in place such as barrier
>>>> placement and group size nuances.
>>> 
>>> Presumably that will remove the hand-made code eventually?
>> 
>> Yes. I just want to be able to expose this interesting functionality in the mean time.
> 
> Makes sense.
> 
>> 
>>> 
>>>> 
>>>> In the current Kaveri hardware version of Okra, it uses group size 256
>>>> by default and this reduce code is designed to work only with 256.
>>>> Also, because it is a handmade kernel, it only runs with
>>>> -UseHSAILDeoptimization and -UseCompressedOops.
>>>> In the included reduce tests, they will skip if the correct flags are
>>>> not there for the tests to run correctly.
>>>> 
>>>> This webrev also changes the way the kernel args are processed to look
>>>> at the actual args type rather than by the type signature of the
>>>> kernel method.
>>>> Regards,
>>>> Eric