[PATCH] D20090: [OPENCL] Fix wrongly vla error for OpenCL array.

Anastasia Stulova via cfe-commits cfe-commits at lists.llvm.org
Thu May 12 10:09:21 PDT 2016


Anastasia added inline comments.

================
Comment at: lib/Sema/SemaType.cpp:2055
@@ -2054,3 +2054,3 @@
 
-  return S.VerifyIntegerConstantExpression(ArraySize, &SizeVal, Diagnoser,
-                                           S.LangOpts.GNUMode).isInvalid();
+  return S
+      .VerifyIntegerConstantExpression(ArraySize, &SizeVal, Diagnoser,
----------------
pxli168 wrote:
> Anastasia wrote:
> > Formatting looks weird though... may be better to leave the first line unmodified?
> I felt very strange, too. But this is what clang-format gives me.
I see. clang-format doesn't take readability into account unfortunately. :)

Let's just leave the first line of this change unmodified at least to make it more readable.

================
Comment at: test/CodeGenOpenCL/vla.cl:1
@@ +1,2 @@
+// RUN: %clang_cc1 -emit-llvm -O0 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL2.0 -DCL20 -o - %s | FileCheck %s --check-prefix=CL20
----------------
pxli168 wrote:
> Anastasia wrote:
> > Could we have a Sema test instead where we accept the VLA if constant AS object is used and give an error otherwise?
> > 
> > Also do we need to test CL2.0? We don't seem to be doing anything different for that version. 
> In earier than OpenCL1.2  program scope variable must reside in constant address space and no
> ```
> const global int
> ```
> can be here.
> But const global value should also not be seen as VLA either.
Why not? Could you write a complete example perhaps? I am not sure I understand your point.

Thanks!


http://reviews.llvm.org/D20090





More information about the cfe-commits mailing list