[SYCL] Minor fixes to SYCLConditionalCallOnDevice pass #15915
Conversation
This patch fixes minor issues in impl and tests for SYCLConditionalCallOnDevice pass: - new FCaller function now have attributes from the old one - fix tests to support RelWithDebInfo and Debug modes
dm-vodopyanov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| @@ -42,8 +42,8 @@ entry: | |||
| ret void | |||
| } | |||
|
|
|||
| ; CHECK: declare spir_func void @call_if_on_device_conditionally1_PREFIX_1(ptr, ptr, i32, i32) | |||
| ; CHECK: declare spir_func void @call_if_on_device_conditionally2_PREFIX_2(ptr, ptr, i32, i32) | |||
| ; CHECK: declare spir_func void @call_if_on_device_conditionally1_PREFIX_1(ptr[[VAL1:.*]], ptr[[VAL2:.*]], i32[[VAL3:.*]], i32[[VAL4:.*]]) | |||
There was a problem hiding this comment.
May be a test can be a bit more comprehensive with the actual checks for the attributes.
There was a problem hiding this comment.
+1 to this comment. The way the test is written, it would pass even without your patch due to .* regexp which allows an empty string.
Therefore, I suggest that you add a separate test where you check that both argument and function attributes have been correctly copied over
dm-vodopyanov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| @@ -28,7 +28,7 @@ entry: | |||
| ret void | |||
| } | |||
|
|
|||
| ; CHECK: declare spir_func void @call_if_on_device_conditionally_PREFIX_1(ptr, ptr, i32, i32) | |||
| ; CHECK: declare spir_func void @call_if_on_device_conditionally_PREFIX_1(ptr[[VAL1:.*]], ptr[[VAL2:.*]], i32[[VAL3:.*]], i32[[VAL4:.*]]) | |||
There was a problem hiding this comment.
| ; CHECK: declare spir_func void @call_if_on_device_conditionally_PREFIX_1(ptr[[VAL1:.*]], ptr[[VAL2:.*]], i32[[VAL3:.*]], i32[[VAL4:.*]]) | |
| ; CHECK: declare spir_func void @call_if_on_device_conditionally_PREFIX_1(ptr{{.*}}, ptr{{.*}}, i32{{.*}}, i32{{.*}}) |
Nit: no need to capture if you don't use the result
| @@ -42,8 +42,8 @@ entry: | |||
| ret void | |||
| } | |||
|
|
|||
| ; CHECK: declare spir_func void @call_if_on_device_conditionally1_PREFIX_1(ptr, ptr, i32, i32) | |||
| ; CHECK: declare spir_func void @call_if_on_device_conditionally2_PREFIX_2(ptr, ptr, i32, i32) | |||
| ; CHECK: declare spir_func void @call_if_on_device_conditionally1_PREFIX_1(ptr[[VAL1:.*]], ptr[[VAL2:.*]], i32[[VAL3:.*]], i32[[VAL4:.*]]) | |||
There was a problem hiding this comment.
+1 to this comment. The way the test is written, it would pass even without your patch due to .* regexp which allows an empty string.
Therefore, I suggest that you add a separate test where you check that both argument and function attributes have been correctly copied over
This patch fixes minor issues in impl and tests for SYCLConditionalCallOnDevice pass: