small webrev to fix bug in hsail kernel argument logic

[Christian Thalinger]

Thanks for the information.  It might be good to enforce this contract in C++ code, though.

On Feb 6, 2014, at 2:29 PM, Deneau, Tom <tom.deneau at amd.com> wrote:

> Christain --
> 
> You're right, our hsail graal backend is designed for compiling and
> dispatching functions coming from something like IntStream.forEach or
> ObjectStream.forEach. (The java 7 based junit tests don't actually use
> the Stream interface but the interface they do use is similar).
> 
> So for now, the only functions we compile to be hsail kernels must
> either:
> 
>   * have int as a final parameter in which case it gets filled from
>     the workitemid.
> 
>   * have an Object as the final parameter in which case it gets
>     filled from the supplied object array indexed by the workitemid.
> 
> In java 8, the lambdas that satisfy IntConsumer or Consumer<T> meet
> these requirements.
> 
> -- Tom
> 
>> -----Original Message-----
>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>> Sent: Thursday, February 06, 2014 3:35 PM
>> To: Deneau, Tom
>> Cc: Doug Simon; graal-dev at openjdk.java.net
>> Subject: Re: small webrev to fix bug in hsail kernel argument logic
>> 
>> 
>> On Feb 6, 2014, at 4:06 AM, Deneau, Tom <tom.deneau at amd.com> wrote:
>> 
>>> Christian --
>>> 
>>> I'm not sure what is being referred to as fragile here.
>>> 
>>> If it is the logic of not passing a last parameter when it is an int,
>> that has been there all along.
>>> (This webrev just firms up the way it decides whether it is the last
>> parameter or not).
>> 
>> Yes, I know it's not part of this change.
>> 
>>> 
>>> The code that sets up the kernel prologue uses similar logic in that
>>> when it wants to load an int parameter and it is the final int
>>> parameter, it knows that that parameter should not be loaded from the
>> kernel arguments but instead should be set from the hsail workitemabsid
>> instruction.
>> 
>> Maybe the fact that I don't know about the other code involved in this
>> contract makes it look fragile to me.  Is there no other way to get to
>> this method than for IntStream?
>> 
>>> 
>>> -- Tom
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>> Sent: Wednesday, February 05, 2014 9:54 PM
>>>> To: Deneau, Tom
>>>> Cc: Doug Simon; graal-dev at openjdk.java.net
>>>> Subject: Re: small webrev to fix bug in hsail kernel argument logic
>>>> 
>>>> The change looks good but in general this looks fragile:
>>>> 
>>>> 93 void HSAILKernelArguments::do_int() {
>>>> 94   // The last int is the iteration variable in an IntStream, but
>> we
>>>> don't pass it
>>>> 95   // since we use the HSAIL workitemid in place of that int value
>>>> 96   if (isLastParameter()) {
>>>> 97     if (TraceGPUInteraction) {
>>>> 98       tty->print_cr("[HSAIL] HSAILKernelArguments::not pushing
>>>> trailing int");
>>>> 99     }
>>>> 100     return;
>>>> 101   }
>>>> 
>>>> On Feb 5, 2014, at 12:35 PM, Deneau, Tom <tom.deneau at amd.com> wrote:
>>>> 
>>>>> Doug --
>>>>> 
>>>>> The small webrev
>>>>> 
>>>>> http://cr.openjdk.java.net/~tdeneau/graal-webrevs/webrev-kernarg-fix
>>>>> es
>>>>> /webrev/
>>>>> 
>>>>> fixes some problems in the HsailKernelArguments code that caused
>>>>> some crashes with certain kernel argument combinations (not any of
>>>>> the existing junit test cases).]
>>>>> 
>>>>> In addition added some new test cases that would have failed but now
>>>> pass.
>>>>> 
>>>>> -- Tom
>>>> 
>>> 
>>> 
>> 
> 
>