[Openmp-dev] Clang9 warning and compiler error with atomics
Alexey Bataev via Openmp-dev
openmp-dev at lists.llvm.org
Fri Dec 6 05:27:43 PST 2019
Best regards,
Alexey Bataev
6 дек. 2019 г., в 08:24, Jonas Hahnfeld via Openmp-dev <openmp-dev at lists.llvm.org> написал(а):
Am Freitag, den 06.12.2019, 13:15 +0000 schrieb Alexey Bataev:
You described the behavior of trivilly copyable types. If we don't call the constructor/destructor, we may break something in the inner type data, if the constructor/destructor requires some additional actions.
We don't need default constructor, we need defaulted drfault constructor, no copy/move operators and defaulted destructor.
The runtime at first allocates memory for the mapped data (here we need to call default constructor to initialize the data),
Why would we need to call the default constructor here? If the data is not mapped to / tofrom, the "value of the corresponding list item is undefined" (OpenMP 5.0, p318).
That's why we have just a warning to notify the user that there might be some problems.
then bitcopies the data from mapped data to the buffer (need to call copy operator)
If the runtime does copy the data, these two steps together are equivalent to a single invocation of the copy-constructor, aren't they?
Generally speaking, no. Default constructor + assignment operator may lead to a different result than just copy constructor (there can be different functionality). The runtime implements it in the way of default constructor (mem alloc) + copy assignment (memcpy).
Jonas
and when we exit we need to destroy the data in the buffer (need for the defaulted destructor).
Best regards,
Alexey Bataev
6 дек. 2019 г., в 08:02, Jonas Hahnfeld <hahnjo at hahnjo.de> написал(а):
For the constructors, the runtime is doing kind of an implicit trivial copy constructor by memcpy'ing. Could you explain (or hint to an explanation) why we need a default constructor, ie T::T() in my previous example?
https://en.cppreference.com/w/cpp/language/destructor#Trivial_destructor
"A trivial destructor is a destructor that performs no action."
This is guaranteed for "trivially copyable" types. Do you agree that we are fine with a trivial destructor?
Jonas
Am Freitag, den 06.12.2019, 12:54 +0000 schrieb Alexey Bataev:
Yes, we need it. Just I like said before, only fully serializable types are guaranteed to be mapped fully correctly. We don not call constructors/destructors, that's why they must be default.
Best regards,
Alexey Bataev
6 дек. 2019 г., в 07:39, Jonas Hahnfeld <hahnjo at hahnjo.de> написал(а):
I think checkTypeMappable() in lib/Sema/SemaOpenMP.cpp currently checks QualType::isTrivialType(). This implies QualType::isTriviallyCopyableType(), but it additionally also requires a default constructor and no non-trivial default constructor.
Do we actually need guarantees on the default constructors? Please note that "trivially copyable" requires a trivial destructor, so this should be fine from that perspective.
For example, consider attached class T which is trivially copyable (see static_assert). Clang warns because T::T() is non-trivial, but I don't see how this affects the mapping (which is eventually just memcpy'ing the int member).
Jonas
Am Freitag, den 06.12.2019, 02:48 +0000 schrieb Doerfert, Johannes via Openmp-dev:
No error please, except if we know something will break. I'll write up a proposal with more detail tomorrow (hopefully).
--
written from my phone
From: Alexey Bataev <Alexey.Bataev at ibm.com>
Sent: Thursday, December 5, 2019 8:43:50 PM
To: Itaru Kitayama <itaru.kitayama at gmail.com>
Cc: Doerfert, Johannes <jdoerfert at anl.gov>; Guido Juckeland <g.juckeland at hzdr.de>; Jeffrey Kelling <j.kelling at hzdr.de>; Sunita Chandrasekaran <schandra at udel.edu>; openmp-dev at lists.llvm.org <openmp-dev at lists.llvm.org>
Subject: RE: [Openmp-dev] Clang9 warning and compiler error with atomics
The only guarantee is the trivially copyable type. If you insist, I can convert a warning into an error.
Best regards,
Alexey Bataev
5 дек. 2019 г., в 21:41, Itaru Kitayama <itaru.kitayama at gmail.com> написал(а):
That’d be extremely helpful for C++ application developers.
On Fri, Dec 6, 2019 at 11:21 Doerfert, Johannes via Openmp-dev <openmp-dev at lists.llvm.org<mailto:openmp-dev at lists.llvm.org>> wrote:
I think we talk past each other. Let me try to rephrase.
Even if a type is not trivially copyable, as defined by the standard, we can determine sufficient conditions under which the mapping will happen as expected by a reasonable user. That said, we need to define these conditions. Does this make sense?
--
written from my phone
________________________________
From: Alexey Bataev <Alexey.Bataev at ibm.com<mailto:Alexey.Bataev at ibm.com>>
Sent: Thursday, December 5, 2019 7:51:02 PM
To: Doerfert, Johannes <jdoerfert at anl.gov<mailto:jdoerfert at anl.gov>>
Cc: Jeffrey Kelling <j.kelling at hzdr.de<mailto:j.kelling at hzdr.de>>; Narayanaswamy, Ravi <ravi.narayanaswamy at intel.com<mailto:ravi.narayanaswamy at intel.com>>; Sunita Chandrasekaran <schandra at udel.edu<mailto:schandra at udel.edu>>; openmp-dev at lists.llvm.org<mailto:openmp-dev at lists.llvm.org> <openmp-dev at lists.llvm.org<mailto:openmp-dev at lists.llvm.org>>; Guido Juckeland <g.juckeland at hzdr.de<mailto:g.juckeland at hzdr.de>>
Subject: RE: Clang9 warning and compiler error with atomics
"Working" has different meanings here. We don't call constructors, destructors or any copy/move operators. And we say the user about this with this warning. If the user does not need the results of these function, he can ignore the warning and transfer data as is. It may work in some cases and in some it may not (if constructors/destructors are very complex, for example). But we don't know for sure whrn it works and when it is not.
Best regards,
Alexey Bataev
5 дек. 2019 г., в 20:39, Doerfert, Johannes <jdoerfert at anl.gov<mailto:jdoerfert at anl.gov>> написал(а):
My key point is: Is the mapping of the type in question"working" or not? If not, it should be an error, if it is working, no warning, only if we don't know, it should be a warning.
--
written from my phone
________________________________
From: Alexey Bataev <Alexey.Bataev at ibm.com<mailto:Alexey.Bataev at ibm.com>>
Sent: Thursday, December 5, 2019 7:34:50 PM
To: Doerfert, Johannes <jdoerfert at anl.gov<mailto:jdoerfert at anl.gov>>
Cc: Jeffrey Kelling <j.kelling at hzdr.de<mailto:j.kelling at hzdr.de>>; Narayanaswamy, Ravi <ravi.narayanaswamy at intel.com<mailto:ravi.narayanaswamy at intel.com>>; Sunita Chandrasekaran <schandra at udel.edu<mailto:schandra at udel.edu>>; openmp-dev at lists.llvm.org<mailto:openmp-dev at lists.llvm.org> <openmp-dev at lists.llvm.org<mailto:openmp-dev at lists.llvm.org>>; Guido Juckeland <g.juckeland at hzdr.de<mailto:g.juckeland at hzdr.de>>
Subject: RE: Clang9 warning and compiler error with atomics
1. The warning is emitted correctly, there is no bug. It just might be annoying but it warns correctly. I'll just put it under different warning group so it would be easier to disable it.
2. +1 here. It would be good if you could fix this bug. NVPTX backend doesn't support atomic loads for sure and some other instructions too.
Best regards,
Alexey Bataev
5 дек. 2019 г., в 20:05, Doerfert, Johannes <jdoerfert at anl.gov<mailto:jdoerfert at anl.gov>> написал(а):
My two cents,
1) while the standard might not guarantee proper mapping for non trivially copyable types, we should determine under which condition we can guarantee the expected behaviour and not issue a warning expect we have to. The warning should then inform the user why we couldn't and how it can be resolved.
Could anyone open a bug for this as well?
2) atomics should "work". I'll look into this with the caveat that the performance is hardware dependent. as Alexey mentioned, if the hardware doesn't provide direct support we'll need to emulate them which might be even more costly. Nevertheless, it should work and not error out.
Cheers,
Johannes
--
written from my phone
________________________________
From: Alexey Bataev <Alexey.Bataev at ibm.com<mailto:Alexey.Bataev at ibm.com>>
Sent: Thursday, December 5, 2019 5:30:23 PM
To: Jeffrey Kelling <j.kelling at hzdr.de<mailto:j.kelling at hzdr.de>>
Cc: Narayanaswamy, Ravi <ravi.narayanaswamy at intel.com<mailto:ravi.narayanaswamy at intel.com>>; Sunita Chandrasekaran <schandra at udel.edu<mailto:schandra at udel.edu>>; openmp-dev at lists.llvm.org<mailto:openmp-dev at lists.llvm.org> <openmp-dev at lists.llvm.org<mailto:openmp-dev at lists.llvm.org>>; Guido Juckeland <g.juckeland at hzdr.de<mailto:g.juckeland at hzdr.de>>; Doerfert, Johannes <jdoerfert at anl.gov<mailto:jdoerfert at anl.gov>>
Subject: RE: Clang9 warning and compiler error with atomics
There is std::is_trivially_copyable trait, you can check if your types are actually trivially copyable.
std::is_trivially_copyable<type>::value is true if type is trivially copyable, false otherwise.
Here is the definition of trivially copyable types
https://en.cppreference.com/w/cpp/named_req/TriviallyCopyable
Compiler relies on the standard here.
Best regards,
Alexey Bataev
5 дек. 2019 г., в 18:03, Jeffrey Kelling <j.kelling at hzdr.de<mailto:j.kelling at hzdr.de>> написал(а):
Hello Alexey.
The warning is definitely fired only for non-trivially copyable
types. But for each of them.
I am not completely sure what the definition of non-trivially copyable is, but
I would not expect the following:
* It is giving me warning for std::tuple:
warning: Non-trivial type 'std::__1::tuple<unsigned int *, unsigned int *,
unsigned int *, unsigned long>' is mapped, only trivial types are guaranteed
to be mapped correctly [-Wopenmp-target]
Tuple has a defaulted copy constructor according to the standard. However, the
example above does have pointers, so it may still warrant a warning, but it
would be nice if I could turn that one off separately.
* I also get a waring on another type:
warning: Non-trivial type 'const
alpaka::vec::Vec<std::__1::integral_constant<unsigned long, 1>, unsigned
long>' is mapped, only trivial types are guaranteed to be mapped correctly [-
Wopenmp-target]
This one also has default move and copy constructors. It only has one data
member:
TVal m_data[TDim::value == 0u ? 1u : TDim::value]; // TVal = unsigned long
While I do agree, that this type is anything bus trivial as a whole, it still
should not warrant this warning.
What I can do is to put this warning into a
different warning group, like -Wopenmp-mapping or something like this.
This would help a lot already, thanks.
Best Regards,
Jeffrey
Best regards,
Alexey Bataev
5 дек. 2019 г., в 16:49, Jeffrey Kelling <j.kelling at hzdr.de<mailto:j.kelling at hzdr.de>> написал(а):
Hello Alexey.
1. What is the preferrable way of fixing the problem with the warning? I
don't think we should throw it away because it may help developers to
identify possible prpblems with the offloading.
I agree, that the warning may be important, which is why it would be nice
it
it would not be spammed so much. I would suggest to not show the warning
if
the type is trivially copyable.
I some cases a warning might be useful if the type, even if trivially
copyable, has member of pointer of reference types. However, there should
be
separate flag to toggle this one (at least a no- flag), because I am
intentionally pushing pointer to device memory into the target region
this way
and thus the spam would continue. Maybe, if there is a reference member
the
warning would always make sense, because the reference can maybe only
refer to
something on the host.
2. It is known problem and
there is no quick way to fix it. Some of the atomic constructs are not
supported by the NVPTX backend. Possible solution is to use critical
sections or, maybe, intrinsics/builtins directly.
This is a good thing to note, thanks.
Best Regards,
Jeffrey Kelling
Best regards,
Alexey Bataev
5 дек. 2019 г., в 15:27, Narayanaswamy, Ravi
<ravi.narayanaswamy at intel.com<mailto:ravi.narayanaswamy at intel.com>> написал(а):
+Alexey
From: Sunita Chandrasekaran [mailto:schandra at udel.edu<mailto:schandra at udel.edu>]
Sent: Wednesday, December 4, 2019 7:02 PM
To: openmp-dev at lists.llvm.org<mailto:openmp-dev at lists.llvm.org>
Cc: Jeffrey Kelling <j.kelling at hzdr.de<mailto:j.kelling at hzdr.de>>; Guido Juckeland
<g.juckeland at hzdr.de<mailto:g.juckeland at hzdr.de>>; Narayanaswamy, Ravi
<ravi.narayanaswamy at intel.com<mailto:ravi.narayanaswamy at intel.com>>; Doerfert, Johannes Rudolf
<jdoerfert at anl.gov<mailto:jdoerfert at anl.gov>> Subject: Clang9 warning and compiler error with
atomics
Hello Everyone
I guess I have not posted to this alias in the near past, but I
probably
*can* ! Guess we will find out :-)
(copying Ravi and Johannes just in case my mail doesn't get thru the
mailing list..please let me know)
Some of us are working on an application PIConGPU which is also one of
the
8 ORNL CAAR projects for Frontier system. There are 2 issues pointed
out
by the developer Jeffrey Kelling (copied here) while he is running
Alpaka
tests with LLVM.
1. Clang9 is spamming the a pointless warning:
warning: Non-trivial type 'const
alpaka::vec::Vec<std::__1::integral_constant<unsigned long, 1>,
unsigned
long>' is mapped, only trivial types are guara
nteed to be mapped correctly [-Wopenmp-target]
While we are aware that the warning will appear, we need to turn off
-werror because of this, may miss more important warnings due to the
spam. Jeff also does not want to turn of -Wopenmp-target, because these
should be the most interesting warnings. It would be nice, if this one
could be fixed.
2. Jeff also just encountered an internal compiler error with atomics
and
the NVPTX backend, and created a bug report:
https://bugs.llvm.org/show_bug.cgi?id=44219
Addressing these issues for CAAR is very time critical and that led me
to
send an email to this mailing list to get some quick advice/insights
into
both these issues.
Thank you very much for your time!!
Cheers
sunita
----------------------------------------
Sunita Chandrasekaran
Asst. Prof. Computer and Information Sciences
Affiliated, Center for Bioinformatics and Computational Biology
Affiliated, Data Science Institute
University of Delaware
430 Smith Hall, Newark, DE, USA
p: 302-831-2714 e: schandra at udel.edu<mailto:schandra at udel.edu>
----------------------------------------
Adjunct Prof. Dept. of Computer Science
University of Houston, TX
----------------------------------------
CRPL
Research Group:
http://crpl.cis.udel.edu/
t: https://twitter.com/sunitachandra29
w: https://www.eecis.udel.edu/~schandra/
--
Dr. Jeffrey Kelling
Computational Science Group (FWCC)
Department of Information Services and Computing (FWC)
Building 613, Room 260
Phone: +49 - 351 - 260 3680
Helmholtz-Zentrum Dresden-Rossendorf e.V.
http://www.hzdr.de
Vorstand: Prof. Dr. Dr. h. c. Roland Sauerbrey
Vereinsregister: VR 1693 beim Amtsgericht Dresden
<smime.p7s>
--
Dr. Jeffrey Kelling
Computational Science Group (FWCC)
Department of Information Services and Computing (FWC)
Building 613, Room 260
Phone: +49 - 351 - 260 3680
Helmholtz-Zentrum Dresden-Rossendorf e.V.
http://www.hzdr.de
Vorstand: Prof. Dr. Dr. h. c. Roland Sauerbrey
Vereinsregister: VR 1693 beim Amtsgericht Dresden
<smime.p7s>
_______________________________________________
Openmp-dev mailing list
Openmp-dev at lists.llvm.org<mailto:Openmp-dev at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/openmp-dev
_______________________________________________
Openmp-dev mailing list
<mailto:Openmp-dev at lists.llvm.org>
Openmp-dev at lists.llvm.org
<https://lists.llvm.org/cgi-bin/mailman/listinfo/openmp-dev>
https://lists.llvm.org/cgi-bin/mailman/listinfo/openmp-dev
<map.cpp>
<signature.asc>
<signature.asc>
_______________________________________________
Openmp-dev mailing list
Openmp-dev at lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/openmp-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/openmp-dev/attachments/20191206/1db28fa5/attachment-0001.html>
More information about the Openmp-dev
mailing list