Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[metal] Refactor sparse shader impl in prep for pointer SNode #1994

Merged
merged 2 commits into from
Oct 26, 2020

Conversation

k-ye
Copy link
Member

@k-ye k-ye commented Oct 25, 2020

Hi,

This PR contains mostly refactors and tweaks needed for pointer.

  • Make NodeManagerData:: ElemIndex a strong type instead of an alias for int32_t. This is because we have to shift the raw int32_t index allocated from NodeManagerData's lists (aka ListManagers), in order for the spinning memory allocation ([Metal] Support dynamic memory allocation on GPU #1174) to work. Using a plain int32_t is dangerous, as I have forgotten about the shift in allocation or recycle previously...
  • Pull out ListgenElement::coords into a separate type: ElementCoords. The generated struct-for kernel only needs to know the coordinates, but not other info inside ListgenElement (which will be expanded later).

Related issue = #1740, #1174

[Click here for the format server]


@k-ye k-ye requested a review from yuanming-hu October 25, 2020 14:15
@k-ye k-ye requested a review from taichi-gardener October 25, 2020 14:17
ElementCoords coords;
// Memory offset from a given address.
// * If in_root_buffer() is true, this is from the root buffer address.
// * O.W. this is from the |id|-th NodeManager's |elem_idx|-th element.
Copy link
Member Author

@k-ye k-ye Oct 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The O.W. part is not implemented in this PR yet (therefore the placeholder in_root_buffer() always returns true at this point).

@codecov
Copy link

codecov bot commented Oct 25, 2020

Codecov Report

Merging #1994 into master will increase coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1994      +/-   ##
==========================================
+ Coverage   43.41%   43.50%   +0.09%     
==========================================
  Files          45       45              
  Lines        6268     6266       -2     
  Branches     1109     1109              
==========================================
+ Hits         2721     2726       +5     
+ Misses       3373     3367       -6     
+ Partials      174      173       -1     
Impacted Files Coverage Δ
python/taichi/lang/kernel.py 73.00% <0.00%> (+1.60%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4cba15a...75e3c7d. Read the comment docs.

Copy link
Member

@yuanming-hu yuanming-hu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, LGTM!

taichi/backends/metal/codegen_metal.cpp Show resolved Hide resolved
taichi/backends/metal/shaders/runtime_structs.metal.h Outdated Show resolved Hide resolved
@k-ye k-ye merged commit 16af509 into taichi-dev:master Oct 26, 2020
@k-ye k-ye deleted the ptr-prep branch October 26, 2020 11:07
@yuanming-hu yuanming-hu mentioned this pull request Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants