Skip to content

[SYCL] Add intelfpga vendor header files #511

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

Merged
merged 1 commit into from
Aug 18, 2019

Conversation

domiyan
Copy link
Contributor

@domiyan domiyan commented Aug 14, 2019

With FPGA header files, users have shortcuts to access FPGA device selector and FPGA extension (fpga_reg).

Example of using FPGA device selector.
#include <CL/sycl/intel/fpga_extensions.hpp>
...
cl::sycl::queue deviceQueue(cl::sycl::intel::fpga_selector{}); // this force fpga hardware device
...

Example of using FPGA extensions.
#include <CL/sycl/intel/fpga_extensions.hpp>
...
r[k] = cl::sycl::intel::fpga_reg(a[k]) + b[k];
...
Signed-off-by: Domi Yan [email protected]

@domiyan domiyan requested a review from bader August 14, 2019 14:55
@domiyan domiyan changed the title Add intelfpga vendor header files [SYCL] Add intelfpga vendor header files Aug 14, 2019
@bader
Copy link
Contributor

bader commented Aug 17, 2019

@domiyan, please, sign new commits.

@domiyan domiyan force-pushed the add_intelfpga_vendor_header_files branch from ee66b62 to 897a620 Compare August 17, 2019 18:07
@domiyan
Copy link
Contributor Author

domiyan commented Aug 17, 2019

@bader. Not sure how to modify sign on the exiting one. I created a new branch, applied the diff and force-pushed.

@bader bader merged commit e438d2b into intel:sycl Aug 18, 2019
Copy link
Contributor

@keryell keryell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few things to clean up further.


#pragma once
#include <CL/sycl/intel/fpga_device_selector.hpp>
#include <CL/sycl/intel/fpga_reg.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing EOL.

#include <CL/sycl/intel/fpga_device_selector.hpp>
...
// force FPGA emulation device
cl::sycl::queue deviceQueue(cl::sycl::intel::fpga_emulator_selector{});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are using {}, replace the outer parenthesis too.

@@ -0,0 +1,31 @@
# FPGA reg

Intel FPGA extension fpga_reg() is implemented in header file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fpga_reg() → fpga_reg()

Intel FPGA extension fpga_reg() is implemented in header file
`#include <CL/sycl/intel/fpga_extensions.hpp>`.

fpga_reg is used to help compiler infer that at least one register is on the corresponding data path.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fpga_reg() → fpga_reg()


## Implementation

The implementation is a wrapper class to map fpga_reg function call to built-in \_\_builtin_intel_fpga_reg()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fpga_reg → fpga_reg()
__builtin_intel_fpga_reg() → __builtin_intel_fpga_reg()

namespace sycl {
namespace intel {

class platform_selector : public default_selector {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this derive from device_selector instead of default_selector?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. I will fix this in next patch.

iclsrc pushed a commit that referenced this pull request Apr 2, 2025
This was added in OpenACC PR #511 in the 3.4 branch.  From an AST/Sema
perspective this is pretty trivial as the infrastructure for 'if'
already exists, however the atomic construct needed to be taught to take
clauses.  This patch does that and adds some testing to do so.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants