Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/15105 )
Change subject: WIP: Asynchronous code generation ...................................................................... Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/15105/9/be/src/codegen/codegen-fn-ptr.h File be/src/codegen/codegen-fn-ptr.h: http://gerrit.cloudera.org:8080/#/c/15105/9/be/src/codegen/codegen-fn-ptr.h@41 PS9, Line 41: static constexpr std::memory_order mem_order_ = std::memory_order_acq_rel; > Also I think I can't use memory_order_acq_rel, I need to use acquire for th Yeah, I just checked it, seems like memory_order_acq_rel is only for read-modify-write operations. http://gerrit.cloudera.org:8080/#/c/15105/9/be/src/codegen/codegen-fn-ptr.h@41 PS9, Line 41: static constexpr std::memory_order mem_order_ = std::memory_order_acq_rel; > I don't quite understand the beginning: "From the perspective of the C++ me To have synchronizes-with you need two atomic operations (e.g. store and load, mutex unlock-lock) with proper memory orderings, e.g. rel/acq, or seq_cst on both sides. Synchronizes-with gives you "happens before" semantics between the operations. "Happens before" is a transitive relationship: A <h-b> B and B <h-b> C ==> A <h-b> C. That's why the following works: int A=0; =================================================== T1 T2 =================================================== A=1 B.store(42, release) if (B.load(acquire) == 42) { // see 42, the load synchronizes with the store cout << A; } T2 can only print 1. But if you do a store(42, release) in T1 and in T2 you do a load(relaxed) even if the load sees the value 42 there will be no "synchronizes with" relationship between the two operations. And if you don't have synchronizes-with then you don't have "happens before" either. E.g.: int A=0; =================================================== T1 T2 =================================================== A=1 B.store(42, release) if (B.load(relaxed) == 42) { // see 42, but there is no synchronizes-with cout << A; } T2 is allowed to print 0 or 1. On top of that, based on the C++ memory model even dependent loads are not guaranteed to work. -- To view, visit http://gerrit.cloudera.org:8080/15105 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7cbfa7c6734dcf03641629429057d6a4194aa6b Gerrit-Change-Number: 15105 Gerrit-PatchSet: 9 Gerrit-Owner: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Fri, 14 Feb 2020 16:32:20 +0000 Gerrit-HasComments: Yes
