Open
Conversation
Contributor
Author
|
launch jenkins |
Contributor
Author
|
launch jenkins |
Stagno
suggested changes
Nov 11, 2020
Contributor
Stagno
left a comment
Stagno
left a comment
There was a problem hiding this comment.
Small changes requested, overall it's ok.
Do we understand now how we could get into this unresolvable race condition, though?
dawn/dawn/src/dawn/Optimizer/PassFieldVersioning.cpp
Lines 240 to 249 in 44770e6
I'm still doubtful about that, because dawn will bail out, but a correctly placed kernel-split can always solve a race condition. If it's not possible anymore to reach that condition, then it's dead code.
| /// @brief Insert a new node | ||
| Vertex& insertNode(int ID) { | ||
| auto [iter, inserted] = vertices_.emplace(ID, Vertex{adjacencyList_.size(), ID}); | ||
| Vertex& insertNode(int Value) { |
Contributor
There was a problem hiding this comment.
Suggested change
| Vertex& insertNode(int Value) { | |
| Vertex& insertNode(int value) { |
we Capitalize only public members of classes
| }; | ||
|
|
||
| protected: | ||
| // map of Value (i.e. normally accessID to Vertex object |
Contributor
There was a problem hiding this comment.
Suggested change
| // map of Value (i.e. normally accessID to Vertex object | |
| // map from accessID to Vertex object |
Isn't this always the case?
Also the description of Vertex::Value below could be improved ... doesn't it always correspond to an accessID?
Comment on lines
43
to
+46
| for(int k = 0+0; k <= ( m_k_size == 0 ? 0 : (m_k_size - 1)) + 0+0; ++k) { | ||
| for(auto const& loc : getEdges(LibTag{}, m_mesh)) { | ||
| m_c_0(deref(LibTag{}, loc), (k + 0)) = m_c(deref(LibTag{}, loc), (k + 0)); | ||
| } }}{ | ||
| for(int k = 0+0; k <= ( m_k_size == 0 ? 0 : (m_k_size - 1)) + 0+0; ++k) { | ||
| for(auto const& loc : getEdges(LibTag{}, m_mesh)) { | ||
| m_a(deref(LibTag{}, loc), (k + 0)) = ((m_b(deref(LibTag{}, loc), (k + 0)) / m_c_0(deref(LibTag{}, loc), (k + 0))) + (::dawn::float_type) 5); | ||
| m_a(deref(LibTag{}, loc), (k + 0)) = ((m_b(deref(LibTag{}, loc), (k + 0)) / m_c(deref(LibTag{}, loc), (k + 0))) + (::dawn::float_type) 5); | ||
| } for(auto const& loc : getEdges(LibTag{}, m_mesh)) { |
Contributor
There was a problem hiding this comment.
This test can be removed then. It was testing that a versioned field was generated, but it's not generated anymore.
Contributor
|
launch jenkins |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Technical Description
Fixes the field version for #1056
The problem is caused by the fact that the algorithm is trying to also fix the cases where the graph does not contains outdegree=0 nodes.
dawn/dawn/src/dawn/Optimizer/PassFieldVersioning.cpp
Line 226 in 30ba6f1
However this can be a normal case, like the example of #1056
Here we fix it by accepting that the graph (that is not a DAG) does not have to contain nodes with outdegree=0 or indegree=0.
Instead we fixed the algorithms that require this condition (see ReadBeforeWriteConflict.cpp)
Resolves / Enhances
#1056
Example
(again) example is in #1056