small webrev to fix bug in hsail kernel argument logic

[Christian Thalinger]

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