[SYCL] Add clang support for FPGA kernel attribute scheduler_target_fmax_mhz

[schittir]

[[intelfpga::scheduler_target_fmax_mhz(N)]] applies to a device function/lambda function. Indicates that the kernel should be pipelined so as to achieve the specified target clock frequency (Fmax) of N MHz. The argument N may be a template parameter. Limits of N are [0,1048576]; This attribute should be ignored for the FPGA emulator device.

This patch should

• Add clang support for attribute
• Add a code-gen test
• Add a sema/AST test

[Fznamznon]

Is there any CodeGen changes are coming?
I guess if the attribute is intended to give some information to FPGA backend, we need to add some info to IR as well.

[schittir]

Is there any CodeGen changes are coming?
I guess if the attribute is intended to give some information to FPGA backend, we need to add some info to IR as well.

Yes. Added a test to show the appropriate IR changes per spec.

[keryell]

I am scared by looking at all these random attributes landing...
Has anyone explained the pure C++ SYCL philosophy to the FPGA teams working on the programming models?
All these should be provided by some C++ library constructs and these tests like if (TargetFmaxMhz > UpperLimit) should be done there. We cannot have the usual FPGA kitchen sink in the Clang front-end like it is done nowadays in the various FPGA company HLS.
Very unlikely this will be accepted upstream in Clang/LLVM...
While in the FPGA companies the motto is "for any problem we will make a #pragma or an attribute for you" ;-) I think this gives bad publicity for SYCL and FPGA... @mkinsner @jbrodman @Ralender

[mkinsner]

I am scared by looking at all these random attributes landing...
Has anyone explained the pure C++ SYCL philosophy to the FPGA teams working on the programming models?
All these should be provided by some C++ library constructs

Some of the tuning knobs make the most sense as attributes because of where they're attached, but many could also make sense as library abstractions. Everything defined as an attribute so far is a hint, and not a functional assertion / semantic modifier. So it can be argued that these are all pure C++, in that it's safe to ignore them as is usual for unhandled attributes (usually with a warning). Anything that cannot be ignored has intentionally not been defined as an attribute, as far as I know.

Library abstractions are sometimes more invasive than attributes, for example to modify a property of kernel callable. Many of the options to solve that are uglier/more invasive than an attribute. So there is some balance between the two, but a lot of attributes have been defined at this point.

That said, I agree that we should push more to the "library abstraction" style for alignment with C++ philosophy, and for a variety of other reasons. It's important that you're drawing attention to this @keryell. This has revived discussion amongst the people defining these tuning attributes.

Can triSYCL help to motivate a use case where one of these FPGA-specific tuning knobs would be used to target an FPGA, and you would handle without a modified compiler stack (as a lib-only implementation)? That would help to make this case.

[keryell]

Can triSYCL help to motivate a use case where one of these FPGA-specific tuning knobs would be used to target an FPGA, and you would handle without a modified compiler stack (as a lib-only implementation)? That would help to make this case.

Thank you for participating to this discussion.
I have not been active enough on this subject but we are back in the business with https://github.com/triSYCL/sycl through our open-source collaboration.
Of course it requires some changes in the compiler stack, but the idea is to remain less invasive.
Some code example with 3 different extensions used: https://github.com/triSYCL/sycl/blob/sycl/unified/master/sycl/test/xocc_tests/edge_detection/edge_detection.cpp#L98
That said, I understand too that some people prefer to use #pragma and attributes as usual too. So probably 2 approaches might be better than only one.

[keryell]

Please look also to our contribution to mitigate the kitchen sink approach to attributes: https://reviews.llvm.org/D88645