-
Notifications
You must be signed in to change notification settings - Fork 104
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
Fix function signatures for IndexSet builders #411
Conversation
@@ -81,7 +81,8 @@ void buildTypedIndexSetAligned(IndexSet& hiset, | |||
* | |||
****************************************************************************** | |||
*/ | |||
void buildLockFreeBlockIndexset(IndexSet& iset, | |||
void buildLockFreeBlockIndexset(RAJA::TypedIndexSet<RAJA::RangeSegment, |
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.
This one uses RangeSegments only. Please remove other segment types.
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.
Rather than deal with these changes here, shall I move the indexsetbuilders into LULESH?
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.
No. They need work, but I believe folks are using them.
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.
Changing them to use strongly-typed IndexSets with a subset of segment types will break for existing users.
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.
Good point. Leave them as is for now. I don't think there is any additional overhead for not using segment types.
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.
👍
@@ -97,7 +98,8 @@ void buildLockFreeBlockIndexset(IndexSet& iset, | |||
* | |||
****************************************************************************** | |||
*/ | |||
void buildLockFreeColorIndexset(IndexSet& iset, | |||
void buildLockFreeColorIndexset( RAJA::TypedIndexSet<RAJA::RangeSegment, |
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.
This one uses RangeSegment and List Segment only. Please remove stride segment type.
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.
That won't fix the bug, because LULESH is still using IndexSet.
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.
I know. We should change the type in lulesh. It should be easy because I think it is using a macro or typedef.
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.
👍
These were missed when TypedIndexSet was created.