[PATCH] D21567: [OpenCL] Generate struct type for sampler_t and function call for the initializer

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.

================
Comment at: lib/CodeGen/CodeGenModule.cpp:2399
@@ +2398,3 @@
+  // therefore no need to be translated.
+  if (getLangOpts().OpenCL && ASTTy->isSamplerT())
+    return;
----------------
Could we lift this check right to the beginning of the function?

================
Comment at: lib/Sema/SemaInit.cpp:6917-6929
@@ -6915,4 +6916,15 @@
             << SourceType;
-      } else if (Entity.getKind() != InitializedEntity::EK_Variable) {
-        llvm_unreachable("Invalid EntityKind!");
+          break;
+        } else if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Init)) {
+          auto Var = cast<VarDecl>(DRE->getDecl());
+          if (!Var->hasGlobalStorage()) {
+            CurInit = ImplicitCastExpr::Create(S.Context, Step->Type,
+                                               CK_LValueToRValue, CurInit.get(),
+                                               /*BasePath=*/nullptr, VK_RValue);
+            break;
+          }
+          Init = cast<ImplicitCastExpr>(const_cast<Expr*>(
+            Var->getInit()))->getSubExpr();
+          SourceType = Init->getType();
+        }
       }
----------------
It would be nice to put some comments here summarizing your description!

Btw, I don't think it's covered in testing yet, isn't it?

================
Comment at: lib/Sema/SemaInit.cpp:6918
@@ +6917,3 @@
+          break;
+        } else if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Init)) {
+          auto Var = cast<VarDecl>(DRE->getDecl());
----------------
I don't get why this level of indirection is added? Should the variable initialization be handled elsewhere?

================
Comment at: lib/Sema/SemaInit.cpp:6922
@@ +6921,3 @@
+            CurInit = ImplicitCastExpr::Create(S.Context, Step->Type,
+                                               CK_LValueToRValue, CurInit.get(),
+                                               /*BasePath=*/nullptr, VK_RValue);
----------------
CurInit.get() can be changed to Init

================
Comment at: test/SemaOpenCL/sampler_t.cl:55
@@ -30,2 +54,2 @@
 
-sampler_t bad(); //expected-error{{declaring function return value of type 'sampler_t' is not allowed}}
+sampler_t bad(void); //expected-error{{declaring function return value of type 'sampler_t' is not allowed}}
----------------
Why was this change required?


https://reviews.llvm.org/D21567