-
Notifications
You must be signed in to change notification settings - Fork 768
Fix potential problems with optional arguments on ReqdWorkGroupAttr #3175
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
Comments
Those test cases does not work with current implementation (existing bug). struct TRIFuncObjGood11 { struct TRIFuncObjGood3 { |
This patch 1. refactors SYCL kernel function attribute reqd_work_group_size to better fit for community standards. 2. refactors the way we handled duplicate attributes logic with when present on a given declaration. 3. refactors the way we handled mutually exclusive attributes logic with when present on a given declaration with reqd_work_group_size, max_work_group_size, max_global_work_dim, and num_simd_work_items attributes. 5. handles redeclarations or template instantiations properly. 6. stores ConstantExpr into the semantic attribute rather than calling getIntegerConstantExpr() for attribute dimension values. 7. fixes crash with optional arguments on ReqdWorkGroupAttr and adds missing diagnostics for merging cases when present on a given declaration with reqd_work_group_size, max_work_group_size, max_global_work_dim, and num_simd_work_items attributes. 8. updates clang-tidy build after reqd_work_group_size attribute changes. 9. adds tests. This patch fixes #5736, #5735, #5734, #5732, #5731, #3223, #3361, #3175, #3742, and #5733 Signed-off-by: Soumi Manna <[email protected]>
One thing to double-check is whether
YDimVal
andZDimVal
can ever be null when we create the default values fromSetDefaultValue()
. I don't think they can be null within thisif
block, but it seems plausible they could be null in theD->getAttr<WorkGroupAttr>()
block below (which could be an existing bug).Originally posted by @AaronBallman in #3170 (comment)
The text was updated successfully, but these errors were encountered: