I will comment - as one of the few people actually working on llvm's atomic implementation with any regularity - that I am opposed to extending the instructions without a strong motivating case. I don't care anywhere near as much about metadata based schemes, but extending the instruction semantics imposes a much larger burden on the rest of the community. That burden has to be well justified and supported. Philip> On Jun 25, 2016, at 7:38 PM, Mehdi Amini via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > >> On Jun 25, 2016, at 11:05 AM, Zhuravlyov, Konstantin <Konstantin.Zhuravlyov at amd.com> wrote: >> >> We believe that it would be best that this is added to the LLVM IR atomic memory instruction as fields on atomic instructions rather than using meta data. >> >> The reasoning is that this information is similar to other information that is represented as instruction fields. For example, the indication that memory operations are atomic rather than non-atomic, the memory ordering of atomics, and whether per-thread or system scope. In all these cases this information has a semantic meaning for the instructions that can be exploited by optimizations. Representing it as meta data would mean this information could be dropped making the optimizations impossible with very significant performance penalty. >> >> For example, if memory operations were not marked as being atomic, all memory operations would have to be generated as sequential consistent atomics at system scope. Although this "default" behavior is correct, it would not be very performant. Similarly, the memory ordering could use a "default" of sequentially consistent, which again is much less efficient than the weaker orderings. By analogy, the memory scope could also have a "default" of system scope, but that is also not performant when the scope is narrower. >> >> In all these cases this information changes the semantics of the instructions. It affects whether a program is undefined behavior. Using a "default" value leads to those same programs being treated as having defined behavior (for example by eliminating data races). > > It is not clear to me if there is any correctness issues to dropping metadata? > >> >> Currently the atomic memory instructions have the own-thread/system recorded on the instruction which is a limited form of memory scope. The proposal is to replace this with a more general field that can have more than 2 values. Languages that do not use memory scopes can simply use the value corresponding to system scope. >> >> We understand that it is good to avoid adding information to LLVM instructions that is not primary, but in this case it seems that the atomicity, memory ordering and memory scope are all equally primary information that characterize the semantics of memory instructions. >> >> We have posted reviews that implement the proposal and invite everyone to discuss it: >> http://reviews.llvm.org/D21723 >> http://reviews.llvm.org/D21724 > > It seems you’re going back to integer, which I don’t really like for reasons mentioned earlier in this thread, and that I don’t feel you addressed here. > > — > Mehdi > > > > >> >> Thank you, >> Konstantin >> >> >> >> -----Original Message----- >> From: Tom Stellard [mailto:tom at stellard.net] >> Sent: Wednesday, June 22, 2016 4:51 PM >> To: Pekka Jääskeläinen <pekka.jaaskelainen at tut.fi> >> Cc: Mehdi Amini <mehdi.amini at apple.com>; Liu, Yaxun (Sam) <Yaxun.Liu at amd.com>; Ke Bai <kebai613 at gmail.com>; Mekhanoshin, Stanislav <Stanislav.Mekhanoshin at amd.com>; llvm-dev at lists.llvm.org; Tye, Tony <Tony.Tye at amd.com>; Sumner, Brian <Brian.Sumner at amd.com>; Zhuravlyov, Konstantin <Konstantin.Zhuravlyov at amd.com> >> Subject: Re: [llvm-dev] Memory scope proposal >> >> + Brian and Konstantin >> >>> On Wed, May 18, 2016 at 11:18:53AM +0300, Pekka Jääskeläinen wrote: >>> Hi all, >>> >>>> On 02.05.2016 17:46, Tom Stellard via llvm-dev wrote: >>>>>> Why not going with a metadata attachment directly and kill the "singlethread" keyword? Something like: >>>>>> Something like: >>>>>> >>>>>> cmpxchg i32* %addr, i32 42, i32 0 monotonic monotonic, 3, >>>>>> !memory.scope{!42} cmpxchg i32* %addr, i32 42, i32 0 monotonic >>>>>> monotonic, 3, !memory.scope{!43} >>>>>> >>>>>> ... >>>>>> >>>>>> !42 = !{"singlethread"} >>>>>> !43 = !{"L2"} >>>>>> >>>>>> >>>>>> I also avoids manipulating/pruning the global map, and target won't depend on integer to keep bitcode compatibility. >>>> This seems like it will work assuming that promoting something like >>>> "L2" to the most conservative memory scope is legal (this is what >>>> would have to happen if the metadata was dropped). Are there any targets where this type of' >>>> promotion would be illegal? >>> >>> +1 >>> >>> Sorry to enter this discussion so late, but in my opinion this is a >>> very good non-intrusive starting point solution. >>> >>> Implementing it as a MD with the assumption of there being a safe >>> conservative memory scope to fall back to (in case the MD gets >>> stripped off for some reason) converts it to a mere optimization hint >>> without the need to touch LLVM IR instruction semantics. >>> >>> Also, as it's only a MD, if we encounter a need to extend it later >>> towards a more integrated solution (or a mechanism to support targets >>> where this scheme is not feasible), we can more easily do so. >>> >>> BR, >>> -- >>> Pekka > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160703/c13281a8/attachment.html>
On Mon, Jul 4, 2016 at 5:09 AM, Philip Reames via llvm-dev < llvm-dev at lists.llvm.org> wrote:> I will comment - as one of the few people actually working on llvm's > atomic implementation with any regularity - that I am opposed to extending > the instructions without a strong motivating case. I don't care anywhere > near as much about metadata based schemes, but extending the instruction > semantics imposes a much larger burden on the rest of the community. That > burden has to be well justified and supported. >In OpenCL 2.x, two atomic operations on the same atomic object need to have the same scope to prevent a data race. This derives from the definition of "inclusive scope" in OpenCL 2.x. Encoding OpenCL 2.x scope as metadata in LLVM IR would be a problem because there cannot be a "safe default value" to be used when the metadata is dropped. If the "largest" scope is used as the default, then the optimizer must guarantee that the metadata is dropped from every atomic operation in the whole program, or not dropped at all. Hence the original attempt to extend LLVM atomic instructions with a broader scope field. Sameer. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160710/ada27fb1/attachment.html>
Zhuravlyov, Konstantin via llvm-dev
2016-Aug-17 20:29 UTC
[llvm-dev] Memory scope proposal
Hi, I have updated the review here: https://reviews.llvm.org/D21723 As Sameer pointed out, the motivation is: In OpenCL 2.x, two atomic operations on the same atomic object need to have the same scope to prevent a data race. This derives from the definition of "inclusive scope" in OpenCL 2.x. Encoding OpenCL 2.x scope as metadata in LLVM IR would be a problem because there cannot be a "safe default value" to be used when the metadata is dropped. If the "largest" scope is used as the default, then the optimizer must guarantee that the metadata is dropped from every atomic operation in the whole program, or not dropped at all. Thanks, Konstantin From: Sameer Sahasrabuddhe [mailto:sameer at sbuddhe.net] Sent: Sunday, July 10, 2016 4:06 AM To: Philip Reames <listmail at philipreames.com> Cc: Mehdi Amini <mehdi.amini at apple.com>; Liu, Yaxun (Sam) <Yaxun.Liu at amd.com>; Ke Bai <kebai613 at gmail.com>; Mekhanoshin, Stanislav <Stanislav.Mekhanoshin at amd.com>; Sumner, Brian <Brian.Sumner at amd.com>; llvm-dev at lists.llvm.org; Zhuravlyov, Konstantin <Konstantin.Zhuravlyov at amd.com>; Tye, Tony <Tony.Tye at amd.com> Subject: Re: [llvm-dev] Memory scope proposal On Mon, Jul 4, 2016 at 5:09 AM, Philip Reames via llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote: I will comment - as one of the few people actually working on llvm's atomic implementation with any regularity - that I am opposed to extending the instructions without a strong motivating case. I don't care anywhere near as much about metadata based schemes, but extending the instruction semantics imposes a much larger burden on the rest of the community. That burden has to be well justified and supported. In OpenCL 2.x, two atomic operations on the same atomic object need to have the same scope to prevent a data race. This derives from the definition of "inclusive scope" in OpenCL 2.x. Encoding OpenCL 2.x scope as metadata in LLVM IR would be a problem because there cannot be a "safe default value" to be used when the metadata is dropped. If the "largest" scope is used as the default, then the optimizer must guarantee that the metadata is dropped from every atomic operation in the whole program, or not dropped at all. Hence the original attempt to extend LLVM atomic instructions with a broader scope field. Sameer. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160817/0afe85c9/attachment.html>