-
Notifications
You must be signed in to change notification settings - Fork 114
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
Host Implementation of Histogram APIs #1974
Open
danhoeflinger
wants to merge
50
commits into
main
Choose a base branch
from
dev/dhoefling/histogram_CPU_impl
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+266
−32
Open
Changes from all commits
Commits
Show all changes
50 commits
Select commit
Hold shift + click to select a range
7bcd21e
histogram CPU initial implementation
danhoeflinger 955f0e2
atomics histogram implementation
danhoeflinger ebcada6
clang format
danhoeflinger 93dd493
comment about atomics
danhoeflinger 4d95044
first draft of atomic increment (unchecked)
danhoeflinger a7fe8a1
atomics include and fix builtin
danhoeflinger 6be0b7b
large case
danhoeflinger 9e3f16c
fix threshold check
danhoeflinger 0592c15
minor improvements
danhoeflinger 6ac87a0
MSVC fixes
danhoeflinger af5defb
parallelize initialization of openMP temp histograms
danhoeflinger 65c30af
removing unnecessary type casting
danhoeflinger eec36d5
improving accumulation of local histograms (simd)
danhoeflinger 1faece5
Properly using IsVector
danhoeflinger a39accd
typo fix
danhoeflinger 2cd184d
special handling thread zero to use global mem
danhoeflinger b3dd2a0
cleanup
danhoeflinger bff45d9
atomic version removal
danhoeflinger 2c8cc04
Revert "cleanup"
danhoeflinger 790121b
Revert "special handling thread zero to use global mem"
danhoeflinger 0392f23
comments and cleanup
danhoeflinger 4556399
handling coarser grained parallelism
danhoeflinger 2fdea34
undo-ing broken thread restriction in openMP
danhoeflinger a0c016a
lift pattern up to algorithm_impl level
danhoeflinger e87ba02
cleanup unnecessary code
danhoeflinger 9f74810
further cleanup / formatting
danhoeflinger e0ed14c
add grain size todo
danhoeflinger db2a474
more general thread local storage
danhoeflinger 240447e
implement omp on demand tls
danhoeflinger 741f8e3
formatting
danhoeflinger fd310b6
formatting
danhoeflinger 6820340
comments and clarity
danhoeflinger 4798d2c
bugfix for serial impl
danhoeflinger ce8b55f
removing debugging output
danhoeflinger 5d1f6e8
formatting
danhoeflinger 6630018
comment adjustment
danhoeflinger 83a9f81
minor naming / formatting
danhoeflinger a3f1304
formatting
danhoeflinger 14cca58
Address review feedback
danhoeflinger 766f42c
formatting
danhoeflinger 6425d37
address review feedback
danhoeflinger a590cac
address feedback
danhoeflinger dd35fc9
formatting
danhoeflinger 455cf9c
Add comment about using `size()`
danhoeflinger 8b094fe
fixing include errors
danhoeflinger 7e1463e
formatting
danhoeflinger d9d4f08
adding const
danhoeflinger bebba5e
address feedback
danhoeflinger d494601
address feedback
danhoeflinger 0a2847f
rename to __enumerable_thread_local_storage
danhoeflinger File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_bin
return negative value?__brick_histogram
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes,
-1
is returned when the input element does not fall within any histogram bin. The correct behavior is to do nothing and skip this input element.Specification
"Input values that do not map to a defined bin are skipped silently."
I recently looked into expanding the bin helper interface to include a separate function to check bounds, and another to get the bin which assumes it is in bounds. I thought this might provide benefit by reducing the number of branches by 1, but I saw no performance benefit from this change for CPU or GPU. It is still something we could pursue in the future.