[PATCH] D109175: [openmp] Emit deferred diag only when device compilation presents

[Wei Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb283d55c90dd: [openmp] Emit deferred diag only when device 
compilation presents (authored by weiwang).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109175/new/

https://reviews.llvm.org/D109175

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/OpenMP/declare_target_messages.cpp
  clang/test/SemaCUDA/openmp-target.cu


Index: clang/test/SemaCUDA/openmp-target.cu
===
--- clang/test/SemaCUDA/openmp-target.cu
+++ clang/test/SemaCUDA/openmp-target.cu
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -triple x86_64 -verify=expected,dev \
 // RUN:-verify-ignore-unexpected=note \
-// RUN:-fopenmp -fopenmp-version=50 -o - %s
+// RUN:-fopenmp -fopenmp-version=50 
-fopenmp-targets=amdgcn-amd-amdhsa -o - %s
 // RUN: %clang_cc1 -triple x86_64 -verify -verify-ignore-unexpected=note\
-// RUN:-fopenmp -fopenmp-version=50 -o - -x c++ %s
+// RUN:-fopenmp -fopenmp-version=50 
-fopenmp-targets=amdgcn-amd-amdhsa -o - -x c++ %s
 // RUN: %clang_cc1 -triple x86_64 -verify=dev -verify-ignore-unexpected=note\
 // RUN:-fcuda-is-device -o - %s
 
Index: clang/test/OpenMP/declare_target_messages.cpp
===
--- clang/test/OpenMP/declare_target_messages.cpp
+++ clang/test/OpenMP/declare_target_messages.cpp
@@ -1,11 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp45 
-fopenmp -fopenmp-version=45 -fnoopenmp-use-tls -ferror-limit 100 -o - %s
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - 
%s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp -fopenmp-targets=x86_64-apple-macos10.7.0 
-fnoopenmp-use-tls -ferror-limit 100 -o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp5,dev5 
-fopenmp -fopenmp-is-device -fopenmp-targets=x86_64-apple-macos10.7.0 
-aux-triple x86_64-apple-macos10.7.0 -fnoopenmp-use-tls -ferror-limit 100 -o - 
%s
 
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 
-o - %s
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd -fopenmp-is-device -fnoopenmp-use-tls 
-ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd 
-fopenmp-targets=x86_64-apple-macos10.7.0 -fnoopenmp-use-tls -ferror-limit 100 
-o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd -fopenmp-is-device 
-fopenmp-targets=x86_64-apple-macos10.7.0 -fnoopenmp-use-tls -ferror-limit 100 
-o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp45 
-fopenmp-version=45 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp5 
-fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 #pragma omp end declare target // expected-error {{unexpected OpenMP directive 
'#pragma omp end declare target'}}
 
 int a, b, z; // omp5-error {{variable captured in declare target region must 
appear in a to clause}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12609,7 +12609,9 @@
 VDecl->setInitStyle(VarDecl::ListInit);
   }
 
-  if (LangOpts.OpenMP && VDecl->isFileVarDecl())
+  if (LangOpts.OpenMP &&
+  (LangOpts.OpenMPIsDevice || !LangOpts.OMPTargetTriples.empty()) &&
+  VDecl->isFileVarDecl())
 DeclsToCheckForDeferredDiags.insert(VDecl);
   CheckCompleteVariableDeclaration(VDecl);
 }
@@ -14839,7 +14841,9 @@
 DiscardCleanupsInEvaluationContext();
   }
 
-  if (FD && (LangOpts.OpenMP || LangOpts.CUDA || LangOpts.SYCLIsDevice)) {
+  if (FD && ((LangOpts.OpenMP && (LangOpts.OpenMPIsDevice ||
+  !LangOpts.OMPTargetTriples.empty())) ||
+ LangOpts.CUDA || LangOpts.SYCLIsDevice)) {
 auto ES = getEmissionStatus(FD);
 if (ES == Sema::FunctionEmissionStatus::Emitted ||
 ES == Sema::FunctionEmissionStatus::Unknown)


Index: clang/test/SemaCUDA/openmp-target.cu
===
--- clang/test/SemaCUDA/openmp-target.cu
+++ clang/test/SemaCUDA/openmp-target.cu
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -triple x86_64 -verify=expected,dev \
 // RUN:-verify-ignore-unexpected=note \
-// RUN:-fopenmp -fopenmp-version=50 -o - %s
+// RUN:-fopenmp -fopenmp-version=50 -fopenmp-targets=amdgcn-amd-amdhsa -o - %s
 // RUN: %clang_cc1 -triple x86_64 -verify 

[PATCH] D109175: [openmp] Emit deferred diag only when device compilation presents

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109175/new/

https://reviews.llvm.org/D109175

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109175: [openmp] Emit deferred diag only when device compilation presents

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D109175#2989997 , @yaxunl wrote:

> In D109175#2989823 , @weiwang wrote:
>
>> In D109175#2987048 , @jdoerfert 
>> wrote:
>>
>>> In D109175#2986782 , @yaxunl 
>>> wrote:
>>>
 I agree with Johannes and Alexey that deferred diags are only needed when 
 LangOpts.OMPTargetTriples.empty(). However, I am not sure whether it is 
 only needed in device compilation.

 For other offloading languages like CUDA/HIP it is needed in both device 
 and host compilation.
>>>
>>> Technically, we might even want to delay in host only mode for OpenMP, but 
>>> that is something we can revisit (e.g., by dynamically setting a flag based 
>>> on the directives we've seen).
>>> @yaxunl Should we for now check if there is any associated offload job?
>>
>> Shall we go ahead and get this change in and think about more longer term 
>> solution later?
>
> LGTM. This patch should be sufficient to limit deferred diags to OpenMP with 
> offloading. Device compilation is covered by OpenMPIsDevice and host 
> compilation is covered by !LangOpts.OMPTargetTriples.empty(). I will leave 
> the decision to Johannes.

Thanks. @jdoerfert. Could you approve this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109175/new/

https://reviews.llvm.org/D109175

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109175: [openmp] Emit deferred diag only when device compilation presents

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D109175#2989823 , @weiwang wrote:

> In D109175#2987048 , @jdoerfert 
> wrote:
>
>> In D109175#2986782 , @yaxunl wrote:
>>
>>> I agree with Johannes and Alexey that deferred diags are only needed when 
>>> LangOpts.OMPTargetTriples.empty(). However, I am not sure whether it is 
>>> only needed in device compilation.
>>>
>>> For other offloading languages like CUDA/HIP it is needed in both device 
>>> and host compilation.
>>
>> Technically, we might even want to delay in host only mode for OpenMP, but 
>> that is something we can revisit (e.g., by dynamically setting a flag based 
>> on the directives we've seen).
>> @yaxunl Should we for now check if there is any associated offload job?
>
> Shall we go ahead and get this change in and think about more longer term 
> solution later?

LGTM. This patch should be sufficient to limit deferred diags to OpenMP with 
offloading. Device compilation is covered by OpenMPIsDevice and host 
compilation is covered by !LangOpts.OMPTargetTriples.empty(). I will leave the 
decision to Johannes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109175/new/

https://reviews.llvm.org/D109175

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109175: [openmp] Emit deferred diag only when device compilation presents

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D109175#2987048 , @jdoerfert wrote:

> In D109175#2986782 , @yaxunl wrote:
>
>> I agree with Johannes and Alexey that deferred diags are only needed when 
>> LangOpts.OMPTargetTriples.empty(). However, I am not sure whether it is only 
>> needed in device compilation.
>>
>> For other offloading languages like CUDA/HIP it is needed in both device and 
>> host compilation.
>
> Technically, we might even want to delay in host only mode for OpenMP, but 
> that is something we can revisit (e.g., by dynamically setting a flag based 
> on the directives we've seen).
> @yaxunl Should we for now check if there is any associated offload job?

Shall we go ahead and get this change in and think about more longer term 
solution later?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109175/new/

https://reviews.llvm.org/D109175

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109175: [openmp] Emit deferred diag only when device compilation presents

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D109175#2986782 , @yaxunl wrote:

> I agree with Johannes and Alexey that deferred diags are only needed when 
> LangOpts.OMPTargetTriples.empty(). However, I am not sure whether it is only 
> needed in device compilation.
>
> For other offloading languages like CUDA/HIP it is needed in both device and 
> host compilation.

Technically, we might even want to delay in host only mode for OpenMP, but that 
is something we can revisit (e.g., by dynamically setting a flag based on the 
directives we've seen).
@yaxunl Should we for now check if there is any associated offload job?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109175/new/

https://reviews.llvm.org/D109175

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109175: [openmp] Emit deferred diag only when device compilation presents

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

I agree with Johannes and Alexey that deferred diags are only needed when 
LangOpts.OMPTargetTriples.empty(). However, I am not sure whether it is only 
needed in device compilation.

For other offloading languages like CUDA/HIP it is needed in both device and 
host compilation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109175/new/

https://reviews.llvm.org/D109175

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits