[cfe-commits] [Patch] SPIR32/SPIR64 targets in Clang

[Benyei, Guy]

Hi Eli,
Thanks for the review.

Attached a fixed patch, please review.

> +    virtual bool hasFeature(StringRef Feature) const {
> +      return Feature == "spir";
> +    }
>
> What's the point of this?

This one is quite the same as in every virtual architecture, including PNaCl/le32. I guess it is a convinient way to find out if the current target implements this kind of abstraction.

thanks
    Guy Benyei


-----Original Message-----
From: Eli Friedman [mailto:eli.friedman at gmail.com] 
Sent: Tuesday, December 11, 2012 01:18
To: Benyei, Guy
Cc: cfe-commits at cs.uiuc.edu
Subject: Re: [cfe-commits] [Patch] SPIR32/SPIR64 targets in Clang

On Sun, Dec 9, 2012 at 1:30 AM, Benyei, Guy <guy.benyei at intel.com> wrote:
>
> As I've got no response up until now, I'm re-sending this patch - 
> removed some target defines that probably should be defined in the 
> pre-processor initialization.
>
>
>
> I think this is a fairly simple patch.

Sorry about the delay; I was out last week.

+      assert(getTriple().getOS() == llvm::Triple::UnknownOS &&
+        "SPIR target must use unknown OS");
+      assert(getTriple().getEnvironment() ==
llvm::Triple::UnknownEnvironment &&
+        "SPIR target must use unknown environment type");

Don't assert something we don't enforce.  If you really want to enforce it, check before the "new SPIR32TargetInfo(T)".

+    virtual bool hasFeature(StringRef Feature) const {
+      return Feature == "spir";
+    }

What's the point of this?

+    virtual const char *getVAListDeclaration() const {
+      return "";
+    }

How did this sneak in?

Index: test/CodeGenOpenCL/spir32_target.cl
===================================================================
--- test/CodeGenOpenCL/spir32_target.cl	(revision 0)
+++ test/CodeGenOpenCL/spir32_target.cl	(revision 0)
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 %s -triple "spir-unknown-unknown" -emit-llvm -o -
| FileCheck %s
+
+// CHECK: target triple = "spir-unknown-unknown"
+
+typedef struct {
+  char c;
+  void *v;
+  void *v2;
+} my_st;
+
+kernel void foo(global long *arg) {
+  arg[0] = sizeof(my_st);
+//CHECK: store i64 12, i64 addrspace(1)*

We generally prefer to write this sort of test using _Static_Assert or equivalent.

-Eli
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: SPIRtarget3.patch
Type: application/octet-stream
Size: 5976 bytes
Desc: SPIRtarget3.patch
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121211/a894c140/attachment.obj>