[Openmp-dev] Clang9 warning and compiler error with atomics
Jonas Hahnfeld via Openmp-dev
openmp-dev at lists.llvm.org
Fri Dec 6 05:24:04 PST 2019
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).
> 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?
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> 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>
> > > > > > > Sent: Thursday, December 5, 2019 7:51:02 PM
> > > > > > > To: Doerfert, Johannes <jdoerfert at anl.gov>
> > > > > > > Cc: Jeffrey Kelling <j.kelling at hzdr.de>; Narayanaswamy, Ravi <ravi.narayanaswamy at intel.com>; Sunita Chandrasekaran <schandra at udel.edu>;
> > > > > > > openmp-dev at lists.llvm.org <openmp-dev at lists.llvm.org>; Guido Juckeland <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> написал(а):
> > > > > > > >
> > > > > > > >
> > > > > > > > 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>
> > > > > > > > Sent: Thursday, December 5, 2019 7:34:50 PM
> > > > > > > > To: Doerfert, Johannes <jdoerfert at anl.gov>
> > > > > > > > Cc: Jeffrey Kelling <j.kelling at hzdr.de>; Narayanaswamy, Ravi <ravi.narayanaswamy at intel.com>; Sunita Chandrasekaran <schandra at udel.edu>;
> > > > > > > > openmp-dev at lists.llvm.org <openmp-dev at lists.llvm.org>; Guido Juckeland <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> написал(а):
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > 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>
> > > > > > > > > Sent: Thursday, December 5, 2019 5:30:23 PM
> > > > > > > > > To: Jeffrey Kelling <j.kelling at hzdr.de>
> > > > > > > > > Cc: Narayanaswamy, Ravi <ravi.narayanaswamy at intel.com>; Sunita Chandrasekaran <schandra at udel.edu>;
> > > > > > > > > openmp-dev at lists.llvm.org <openmp-dev at lists.llvm.org>; Guido Juckeland <g.juckeland at hzdr.de>;
> > > > > > > > > Doerfert, Johannes <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> написал(а):
> > > > > > > > > >
> > > > > > > > > > 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> написал(а):
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > 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> написал(а):
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > +Alexey
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > From: Sunita Chandrasekaran [mailto:schandra at udel.edu]
> > > > > > > > > > > > > > Sent: Wednesday, December 4, 2019 7:02 PM
> > > > > > > > > > > > > > To: openmp-dev at lists.llvm.org
> > > > > > > > > > > > > > Cc: Jeffrey Kelling <j.kelling at hzdr.de>; Guido Juckeland
> > > > > > > > > > > > > > <g.juckeland at hzdr.de>; Narayanaswamy, Ravi
> > > > > > > > > > > > > > <ravi.narayanaswamy at intel.com>; Doerfert, Johannes Rudolf
> > > > > > > > > > > > > > <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
> > > > > > > > > > > > > > ----------------------------------------
> > > > > > > > > > > > > > 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
> > > > > > > https://lists.llvm.org/cgi-bin/mailman/listinfo/openmp-dev
> > > > >
> > > > > _______________________________________________Openmp-dev mailing listOpenmp-dev at lists.llvm.org
> > > > > https://lists.llvm.org/cgi-bin/mailman/listinfo/openmp-dev
> > > > <map.cpp>
> > > > <signature.asc>
> > <signature.asc>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/openmp-dev/attachments/20191206/a4fd77c0/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.llvm.org/pipermail/openmp-dev/attachments/20191206/a4fd77c0/attachment-0001.sig>
More information about the Openmp-dev
mailing list