[SYCL][FPGA] Allow use_stall_enable_clusters attribute to kernel

[smanna12]

This patch collects and applies the FPGA attribute intel::use_stall_enable_clusters to the callers/SYCL kernel if directly applied through functors/lambda function.

The attribute has to be applicable to all functions, which can include the SYCL kernels and ​must not be propagated up to the caller/SYCL kernel when called from a function.

This patch fixes FPGA emulator bug that was introduced on #3900.

Signed-off-by: Soumi Manna [email protected]

[smanna12]

I have conversation with @mendell27. The attribute intel::use_stall_enable_clusters needs to be applied to the callers/SYCL kernel if directly applied through functors/lambda function.

[erichkeane]

Can you please include a test that shows that your comment in AttrDocs.td is true?

[smanna12]

Can you please include a test that shows that your comment in AttrDocs.td is true?

@erichkeane, Thanks for the review. I have already a test for this for both CodeGen and SemaSYCL:

[[intel::use_stall_enable_clusters]] void test() {}

SemaSYCL:
// Test attribute is not propagated to the kernel.
    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel3
    // CHECK-NOT:   SYCLIntelUseStallEnableClustersAttr {{.*}}
    h.single_task<class test_kernel3>(
        []() { test(); });

CodeGenSYCL:
// CHECK: define {{.*}}spir_kernel void @{{.*}}test_kernel3() #0 !kernel_arg_buffer_location ![[NUM5]]
    // CHECK: define {{.*}}spir_func void @_Z4testv() #3 !stall_enable ![[NUM4]]
    h.single_task<class test_kernel3>(
        []() { test(); });
// CHECK: ![[NUM4]] = !{i32 1}
// CHECK: ![[NUM5]] = !{}

[erichkeane]

Can you please include a test that shows that your comment in AttrDocs.td is true?

@erichkeane, Thanks for the review. I have already a test for this for both CodeGen and SemaSYCL:

[[intel::use_stall_enable_clusters]] void test() {}

SemaSYCL:
// Test attribute is not propagated to the kernel.
    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel3
    // CHECK-NOT:   SYCLIntelUseStallEnableClustersAttr {{.*}}
    h.single_task<class test_kernel3>(
        []() { test(); });

CodeGenSYCL:
// CHECK: define {{.*}}spir_kernel void @{{.*}}test_kernel3() #0 !kernel_arg_buffer_location ![[NUM5]]
    // CHECK: define {{.*}}spir_func void @_Z4testv() #3 !stall_enable ![[NUM4]]
    h.single_task<class test_kernel3>(
        []() { test(); });
// CHECK: ![[NUM4]] = !{i32 1}
// CHECK: ![[NUM5]] = !{}

Based on the diff, you removed at least the sema one!

The test is not showing on the diff because it is an existing test in sema. I did not add any new one for the current diff. So Sema test is still there.

Also, the codegen one is testing that it IS there, what about when it is ignored?

[[intel::use_stall_enable_clusters]] void test() {}

No, it is ignored. The attribute does not get propagated to the kernel below, i.e. spir_kernel.
// CHECK: define {{.*}}spir_kernel void @{{.*}}test_kernel3() #0 !kernel_arg_buffer_location ![[NUM5]]

Here the metadata is attached to the functionDecl that the attribute is applied to test().
 // CHECK: define {{.*}}spir_func void @_Z4testv() #3 !stall_enable ![[NUM4]]

 h.single_task<class test_kernel3>(
      []() { test(); });
// CHECK: ![[NUM5]] = !{}
// CHECK: ![[NUM4]] = !{i32 1}

[smanna12]

Can you please include a test that shows that your comment in AttrDocs.td is true?

@erichkeane, Thanks for the review. I have already a test for this for both CodeGen and SemaSYCL:

[[intel::use_stall_enable_clusters]] void test() {}

SemaSYCL:
// Test attribute is not propagated to the kernel.
    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel3
    // CHECK-NOT:   SYCLIntelUseStallEnableClustersAttr {{.*}}
    h.single_task<class test_kernel3>(
        []() { test(); });

CodeGenSYCL:
// CHECK: define {{.*}}spir_kernel void @{{.*}}test_kernel3() #0 !kernel_arg_buffer_location ![[NUM5]]
    // CHECK: define {{.*}}spir_func void @_Z4testv() #3 !stall_enable ![[NUM4]]
    h.single_task<class test_kernel3>(
        []() { test(); });
// CHECK: ![[NUM4]] = !{i32 1}
// CHECK: ![[NUM5]] = !{}

Based on the diff, you removed at least the sema one!

The test is not showing on the diff because it is an existing test in sema. I did not add any new one for the current diff. So Sema test is still there.

Also, the codegen one is testing that it IS there, what about when it is ignored?

[[intel::use_stall_enable_clusters]] void test() {}

No, it is ignored. The attribute does not get propagated to the kernel below, i.e. spir_kernel.
// CHECK: define {{.*}}spir_kernel void @{{.*}}test_kernel3() #0 !kernel_arg_buffer_location ![[NUM5]]

Here the metadata is attached to the functionDecl that the attribute is applied to test().
 // CHECK: define {{.*}}spir_func void @_Z4testv() #3 !stall_enable ![[NUM4]]

 h.single_task<class test_kernel3>(
      []() { test(); });
// CHECK: ![[NUM5]] = !{}
// CHECK: ![[NUM4]] = !{i32 1}

@erichkeane, i have updated tests. Added comments and showed that If intel::use_stall_enable_clusters is applied to a function called from a device kernel, the attribute is ignored and it is not propagated to the kernel.

[smanna12]

Thanks @erichkeane for the review.

[AaronBallman]

LGTM

[smanna12]

Thanks @AaronBallman for the review and merge.

[AaronBallman]

Sorry for merging before the review from @elizabethandrews arrived! Please address those concerns in a follow-up PR.

[smanna12]

Thanks @elizabethandrews for the reviews. I will address all your comments in a follow-up patch (they are mostly NFC changes).

[smanna12]

Thanks @elizabethandrews for the reviews. I will address all your comments in a follow-up patch (they are mostly NFC changes).

@elizabethandrews, i have addressed all your review comments and created PR is here: #4060