ABataev added inline comments.
================
Comment at: clang/test/OpenMP/target_parallel_for_is_device_ptr_messages.cpp:93
;
-#pragma omp target parallel for private(ps) is_device_ptr(ps) //
expected-error{{private variable cannot be in a is_device_ptr clause in
'#pragma omp target parallel for' directive}} expected-note{{defined as
private}}
+#pragma omp target parallel for private(ps) is_device_ptr(ps)
for (int ii=0; ii<10; ii++)
----------------
jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > jdoerfert wrote:
> > > > > ABataev wrote:
> > > > > > jdoerfert wrote:
> > > > > > > I think this should cause an error or at least a warning. Telling
> > > > > > > the compiler `ps` is a device pointer only to create a local
> > > > > > > uninitialized shadowing variable seems like an error to me.
> > > > > > It is allowed according to OpenMP 5.0. Private copy must be created
> > > > > > in the context of the parallel region, not the target region. So,
> > > > > > for OpenMP 5.0 we should not emit error here.
> > > > > What does that mean and how does that affect my reasoning?
> > > > It means, that for OpenMP 5.0 we should emit a warning/error here. It
> > > > is allowed according to the standard, we should allow it too.
> > > > So, for OpenMP 5.0 we should not emit error here.
> > > > that for OpenMP 5.0 we should emit a warning/error here.
> > >
> > > The last answer contradicts what you said earlier. I expect there is a
> > > *not* missing, correct?
> > >
> > >
> > > Assuming you do not want an error, which is fine, I still think a warning
> > > is appropriate as it seems to me there is never a reason to have a
> > > `is_device_ptr` clause for a private value. I mean, it is either a bug by
> > > the programmer, e.g., 5 letters of `firstprivate` went missing, or simply
> > > nonsensical code for which we warn in other situations as well.
> > Missed `not`.
> > These kind of construct are explicitly allowed in OpenMP. And we should
> > obey the standard unconditionally.
> > Plus, there might be situations where user may require it explicitly. For
> > example, the device pointer is dereferenced in one of the clauses for the
> > subregions but in the deeper subregion it might be used as a private
> > pointer. Why we should emit a warning here?
> If you have a different situation, e.g., the one you describe, you should not
> have a warning. Though, that is not the point. If you have the situation
> above (single directive), as per my reasoning, there doesn't seem to be a
> sensible use case. If you have one and we should create an explicit test for
> it.
>
> > These kind of construct are explicitly allowed in OpenMP.
> `explicitly allowed` != `not forbidded` (yet)
> > And we should obey the standard unconditionally.
> Nobody says we should not. We warn people all the time even if it is valid
> code.
Warnings may prevent successful compilation in some cases, e.g. when warnings
are treated as errors. Why we should emit a warning if the construct is allowed
by the standard? Ask to change the standard if you did not agree with it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65835/new/
https://reviews.llvm.org/D65835
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
