llvm dev - Sep 2012 - [LLVMdev] [pocl-devel] [cfe-dev] SPIR provisional specification is now available in the Khronos website

Carlos,
 AMD's OpenCL implementation(both CPU and GPU) has worked for years with the
way SPIR represents locals. If there is problems with the representation then it
is an implementation issue. One of the issues with using extra kernel arguments
is that it requires extra validation and complexity at the runtime level that is
not needed if it is handled internally by the compiler. That being said, both
ways of doing it are equally valid, but the choice of which way to do it is a
implementation decision. I don't think it would be that difficult to lower
global variables to function arguments given SPIR representation.

Micah
> -----Original Message-----
> From: Carlos Sánchez de La Lama [mailto:csanchezdll at gmail.com]
> Sent: Friday, September 28, 2012 12:34 AM
> To: James Molloy
> Cc: Pekka Jääskeläinen; Ouriel, Boaz; pocl-devel at lists.sourceforge.net;
> Villmow, Micah; cfe-dev at cs.uiuc.edu; llvmdev at cs.uiuc.edu
> Subject: Re: [pocl-devel] [cfe-dev] [LLVMdev] SPIR provisional
> specification is now available in the Khronos website
> 
> Hi guys,
> 
> > So it is valid SPIR, as the specification stands, to manipulate
> > __local variables as Constants in a way that is extremely difficult to
> > undo. That is, in order to transform SPIR to code that can run on a
> > CPU, the GlobalVariable (which is a subclass of Constant) must be
> > replaced with a dynamically calculated Value (which is not a subclass
> of constant).
> 
> What about translating automatic locals to function scope pointers?
> This will make handling of automatic locals and local pointer arguments
> similar, which is desirable as they are just a way to describe the same
> thing (I understand automatic locals as just a simpler way to use local
> buffers than local arguments).
> 
> In fact, pocl converts automatic locals to implicit "extra"
kernel
> arguments and manages both cases the same way.
> 
> Carlos

Micah,

You're saying it works for you, but Clang doesn't currently anywhere
near
the range of horrible constantexpr constructs it is possible to create. You
can "get by" at the moment with just handling ConstantGEPs, because of
the
way Clang works.

But SPIR isn't restricted to Clang, and the problem is that it is
*possible* (although not probable, or nice, but that is irrelevant for
corner conditions) to get valid SPIR that it is *very* difficult to get
into a shape that you can code generate for CPUs.

Even the SAFECode snippet that Pekka noted doesn't even handle the case of
ConstantShuffleVectors, for example.

You can easily simplify this problem with a restriction in SPIR: disallow
ConstantExpr casts - no ptrtoint constant expression. Because
GlobalVariables have pointer type, if you disallow converting their type to
non-pointer type in a constantexpr, the number of constantexpr subclasses
you have to deal with is drastically reduced (to essentially just BitCast
and GEP).

That would be a simple, reasonable restriction that would stop potentially
maliciously horrible test cases causing all CPU SPIR clients to write
upwards of a hundred lines of conversion code.

Cheers,

James

On 28 September 2012 16:48, Villmow, Micah <Micah.Villmow at amd.com>
wrote:
> Carlos,
>  AMD's OpenCL implementation(both CPU and GPU) has worked for years
with
> the way SPIR represents locals. If there is problems with the
> representation then it is an implementation issue. One of the issues with
> using extra kernel arguments is that it requires extra validation and
> complexity at the runtime level that is not needed if it is handled
> internally by the compiler. That being said, both ways of doing it are
> equally valid, but the choice of which way to do it is a implementation
> decision. I don't think it would be that difficult to lower global
> variables to function arguments given SPIR representation.
>
> Micah
>
> > -----Original Message-----
> > From: Carlos Sánchez de La Lama [mailto:csanchezdll at gmail.com]
> > Sent: Friday, September 28, 2012 12:34 AM
> > To: James Molloy
> > Cc: Pekka Jääskeläinen; Ouriel, Boaz; pocl-devel at
lists.sourceforge.net;
> > Villmow, Micah; cfe-dev at cs.uiuc.edu; llvmdev at cs.uiuc.edu
> > Subject: Re: [pocl-devel] [cfe-dev] [LLVMdev] SPIR provisional
> > specification is now available in the Khronos website
> >
> > Hi guys,
> >
> > > So it is valid SPIR, as the specification stands, to manipulate
> > > __local variables as Constants in a way that is extremely
difficult to
> > > undo. That is, in order to transform SPIR to code that can run on
a
> > > CPU, the GlobalVariable (which is a subclass of Constant) must be
> > > replaced with a dynamically calculated Value (which is not a
subclass
> > of constant).
> >
> > What about translating automatic locals to function scope pointers?
> > This will make handling of automatic locals and local pointer
arguments
> > similar, which is desirable as they are just a way to describe the
same
> > thing (I understand automatic locals as just a simpler way to use
local
> > buffers than local arguments).
> >
> > In fact, pocl converts automatic locals to implicit "extra"
kernel
> > arguments and manages both cases the same way.
> >
> > Carlos
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120928/9235b0b8/attachment.html>

James,
Thanks for the suggestion for how to make this case easier to handle. I'll
bring this up to the entire working group in our next meeting.

Micah

From: mankeyrabbit at gmail.com [mailto:mankeyrabbit at gmail.com] On Behalf Of
James Molloy
Sent: Friday, September 28, 2012 9:46 AM
To: Villmow, Micah
Cc: Carlos Sánchez de La Lama; Pekka Jääskeläinen; Ouriel, Boaz; pocl-devel at
lists.sourceforge.net; cfe-dev at cs.uiuc.edu; llvmdev at cs.uiuc.edu
Subject: Re: [pocl-devel] [cfe-dev] [LLVMdev] SPIR provisional specification is
now available in the Khronos website

Micah,

You're saying it works for you, but Clang doesn't currently anywhere
near the range of horrible constantexpr constructs it is possible to create. You
can "get by" at the moment with just handling ConstantGEPs, because of
the way Clang works.

But SPIR isn't restricted to Clang, and the problem is that it is *possible*
(although not probable, or nice, but that is irrelevant for corner conditions)
to get valid SPIR that it is *very* difficult to get into a shape that you can
code generate for CPUs.

Even the SAFECode snippet that Pekka noted doesn't even handle the case of
ConstantShuffleVectors, for example.

You can easily simplify this problem with a restriction in SPIR: disallow
ConstantExpr casts - no ptrtoint constant expression. Because GlobalVariables
have pointer type, if you disallow converting their type to non-pointer type in
a constantexpr, the number of constantexpr subclasses you have to deal with is
drastically reduced (to essentially just BitCast and GEP).

That would be a simple, reasonable restriction that would stop potentially
maliciously horrible test cases causing all CPU SPIR clients to write upwards of
a hundred lines of conversion code.

Cheers,

James
On 28 September 2012 16:48, Villmow, Micah <Micah.Villmow at
amd.com<mailto:Micah.Villmow at amd.com>> wrote:
Carlos,
 AMD's OpenCL implementation(both CPU and GPU) has worked for years with the
way SPIR represents locals. If there is problems with the representation then it
is an implementation issue. One of the issues with using extra kernel arguments
is that it requires extra validation and complexity at the runtime level that is
not needed if it is handled internally by the compiler. That being said, both
ways of doing it are equally valid, but the choice of which way to do it is a
implementation decision. I don't think it would be that difficult to lower
global variables to function arguments given SPIR representation.

Micah
> -----Original Message-----
> From: Carlos Sánchez de La Lama [mailto:csanchezdll at
gmail.com<mailto:csanchezdll at gmail.com>]
> Sent: Friday, September 28, 2012 12:34 AM
> To: James Molloy
> Cc: Pekka Jääskeläinen; Ouriel, Boaz; pocl-devel at
lists.sourceforge.net<mailto:pocl-devel at lists.sourceforge.net>;
> Villmow, Micah; cfe-dev at cs.uiuc.edu<mailto:cfe-dev at
cs.uiuc.edu>; llvmdev at cs.uiuc.edu<mailto:llvmdev at cs.uiuc.edu>
> Subject: Re: [pocl-devel] [cfe-dev] [LLVMdev] SPIR provisional
> specification is now available in the Khronos website
>
> Hi guys,
>
> > So it is valid SPIR, as the specification stands, to manipulate
> > __local variables as Constants in a way that is extremely difficult to
> > undo. That is, in order to transform SPIR to code that can run on a
> > CPU, the GlobalVariable (which is a subclass of Constant) must be
> > replaced with a dynamically calculated Value (which is not a subclass
> of constant).
>
> What about translating automatic locals to function scope pointers?
> This will make handling of automatic locals and local pointer arguments
> similar, which is desirable as they are just a way to describe the same
> thing (I understand automatic locals as just a simpler way to use local
> buffers than local arguments).
>
> In fact, pocl converts automatic locals to implicit "extra"
kernel
> arguments and manages both cases the same way.
>
> Carlos

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120928/819b5a3a/attachment.html>

On 09/28/2012 07:45 PM, James Molloy wrote:
> That would be a simple, reasonable restriction that would stop potentially
> maliciously horrible test cases causing all CPU SPIR clients to write
upwards of
> a hundred lines of conversion code.
Are you proposing to disallow the use of an IR instruction type to *possibly*
avoid problems from the (slight) misuse of another LLVM IR construct?

IMHO there should be a more robust solution that solves the misused construct
instead of just trying to "put out the fires it creates". E.g. some
sort of
"thread local"-type of qualifier for the global which disallows
certain
optimizations. A new linkage type perhaps? Someone more familiar with the
LLVM IR than me might have a better idea.

I understand that adding the automatic local as a kernel argument (in the specs)
is too intrusive now given the existing implementations that do it
otherwise, especially for those for which the semantics "happens" to
be
correct as is. That is, the currently popular GPUs with separate
local/scratchpad memories.

Have a nice weekend,
-- 
--Pekka

On Sep 28, 2012, at 9:45 AM, James Molloy <james at jamesmolloy.co.uk>
wrote:
> You can easily simplify this problem with a restriction in SPIR: disallow
ConstantExpr casts - no ptrtoint constant expression. Because GlobalVariables
have pointer type, if you disallow converting their type to non-pointer type in
a constantexpr, the number of constantexpr subclasses you have to deal with is
drastically reduced (to essentially just BitCast and GEP).
Wouldn't an easier solution just be not to represent them as constants in
the first place?  For instance, you could have a built-in function to get the
address of local N, where N is taken as a parameter.  You can call the builtins
at the beginning of the kernel, and then proceed to use them as you wish without
having to worry about reversing a constant folding later.  Plus, if a given
vendor's backend wants the address to get constant folded, it's easy to
do replaceAllUsesWith of the call with a global, and run an appropriate constant
folding pass.

--Owen
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120928/15ac01c4/attachment.html>