[PATCH] D11361: [OpenMP] Target directive host codegen

[Samuel Antao via cfe-commits]

sfantao added a comment.

Thanks for review. The new diff now uses a proxy function. See other comments inlined.

Thanks again!
Samuel


================
Comment at: lib/CodeGen/CGExpr.cpp:1969-1970
@@ -1945,4 +1968,4 @@
         else
-          return EmitCapturedFieldLValue(*this, CapturedStmtInfo->lookup(VD),
-                                         CapturedStmtInfo->getContextValue());
+          return EmitCapturedValue(*this, CapturedStmtInfo->lookup(VD),
+                                   CapturedStmtInfo->getContextValue());
       }
----------------
ABataev wrote:
> Instead I would do the same thing I did for tasks.
> Generate captured function as is. But also create the second function with the profile required for target codegen. This function must gather all its parameters into a single record and then call auto generated captured function. This captured function must be marked as AlwaysInline.
> In this case you don't need to make some additional changes in Sema for particular processing of target directives, you will just need to generate simple function in codegen.
> This may result in a little bit slower performance, but we may improve it later, when we have time to improve codegen for outlined functions for CapturedDecls.
> I don't like the idea of reinventing of features, that are invented already, like capturing of VLAs, exposing some private functions (like getVLASizeMap()) etc.
> so the code would be like this:
> ```
> void .omp_outlined.(%captures *ptr) always_inline {
>  <captured code>;
> }
> void .target_omp_outlined.(int* a, float* b, ...) {
>   %captures rec;
>   rec.a_ref = a;
>   rec.b_ref = b;
>   ...
>   .omp_outlined.(&rec);
> }
> ```
Ok, I am now using the proxy function.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3065
@@ +3064,3 @@
+      QualType SizeArrayType = CGF.getContext().getConstantArrayType(
+          CGF.getContext().getIntTypeForBitwidth(64, /*Signed=*/true),
+          PointerNumAP, ArrayType::Normal, /*IndexTypeQuals=*/0);
----------------
ABataev wrote:
> Maybe CGF.getContext().getSizeType()?
Done!

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3163
@@ +3162,3 @@
+                                    OffloadingArgs);
+  auto Error = CGF.Builder.CreateICmpNE(Return, CGF.Builder.getInt32(0));
+  CGF.Builder.CreateCondBr(Error, OffloadFailBlock, ContBlock);
----------------
ABataev wrote:
> CGF.Builder.CreateIsNotNull()?
Done!

================
Comment at: test/OpenMP/target_codegen.cpp:1-3
@@ +1,4 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// expected-no-diagnostics
----------------
ABataev wrote:
> It would be good to see the tests for 32 bit target.
Done!


http://reviews.llvm.org/D11361