Tasksequence by rho180 · Pull Request #3 · rho180/oneAPI-samples

rho180 · 2022-07-18T19:34:09Z

Existing Sample Changes

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes Issue#

External Dependencies

List any external dependencies created as a result of this change.

Type of change

Please delete options that are not relevant. Add a 'X' to the one that is applicable.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Implement fixes for ONSAM Jiras

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Delete this line and everything below if this is not a PR for a new code sample

Delete this line and all above it if this PR is for a new code sample

Adding a New Sample(s)

Description

Please include a description of the sample

Checklist

Administrative

Review sample design with the appropriate Domain Expert:
If you have any new dependencies/binaries, inform the oneAPI Code Samples Project Manager

Code Development

Implement coding guidelines and ensure code quality. see wiki for details
Adhere to readme template
Enforce format via clang-format config file
Adhere to sample.json specification. https://github.com/oneapi-src/oneAPI-samples/wiki/sample-json-specification
Ensure/create CI test configurations for sample (ciTests field) https://github.com/oneapi-src/oneAPI-samples/wiki/sample-json-ci-test-object
Run jsonlint on sample.json to verify json syntax. www.jsonlint.com

Security and Legal

OSPDT Approval (see Project Manager for assistance)
Compile using the following compiler flags and fix any warnings, the falgs are: "/Wall -Wformat-security -Werror=format-security"
Bandit Scans (Python only)
Virus scan

Review

Review DPC++ code with Paul Peterseon. (GitHub User: pmpeter1)
Review readme with Tom Lenth(@tomlenth) and/or Project Manager
Tested using Dev Cloud when applicable

Signed-off-by: horobert <horobert@intel.com>

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/CMakeLists.txt

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/sample.json

yuguen · 2022-07-19T08:41:40Z

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/src/CMakeLists.txt

+# 1. The "compile" stage compiles the device code to an intermediate representation (SPIR-V).
+# 2. The "link" stage invokes the compiler's FPGA backend before linking.
+#    For this reason, FPGA backend flags must be passed as link flags in CMake.
+set(EMULATOR_COMPILE_FLAGS "-Wall ${WIN_FLAG} -fintelfpga -DFPGA_EMULATOR -g0")


Any particular reason for adding -g0 here and below?

At the time, there was a bug in the emulator with task_sequence that made it incompatible with anything but g0 (detailed in a release note). I will doublecheck this now as I believe the bug has been fixed.

yuguen · 2022-07-19T08:46:18Z

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/README.md

@@ -0,0 +1,313 @@
+


Just being super pedantic but you can remove these two empty lines

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/README.md

yuguen · 2022-07-19T09:36:49Z

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/src/task_sequence.cpp

+#include <ext/intel/experimental/task_sequence.hpp>
+#include <sycl/ext/intel/fpga_extensions.hpp>
+
+// dpc_common.hpp can be found in the dev-utilities include folder.


Remove the comment on where to find dpc_common

yuguen · 2022-07-19T09:37:35Z

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/src/task_sequence.cpp

+// 's'
+ValueT dotProduct(ValueT* v, size_t s, size_t sz) {
+  int result = 1;
+  for (size_t i = s; i < s + sz; i++) result *= v[i];


This is not a dot product, this is a reduction :)
A dot product would be result += v1[i] * v2[i];

yuguen · 2022-07-19T09:40:23Z

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/src/task_sequence.cpp

+}
+
+// return the absolute value of 'x'
+template <typename T>


Why not use std::abs?

yuguen · 2022-07-19T09:43:56Z

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/src/task_sequence.cpp

+      parallelTime = (double)(end - start) * 1e-6;
+    }
+
+    if (abs(out[0] - golden) < (ValueT)0.001)


Please use { } here and below

yuguen · 2022-07-19T09:44:16Z

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/src/task_sequence.cpp

+    std::cerr << "Caught a SYCL host exception:\n" << e.what() << "\n";
+    std::terminate();
+  }
+}


I really like your coding style, I wish my code was as clean as yours!

yuguen · 2022-07-19T10:19:10Z

As usual, I'll ask you to provide me with SPETC links to your linux and windows runs of the tutorial :)

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/CMakeLists.txt

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/License.txt

yuguen · 2022-10-21T09:19:58Z

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/task_sequence.sln

@@ -0,0 +1,25 @@
+


Please remove this file entirely

yuguen · 2022-10-21T09:23:03Z

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/src/CMakeLists.txt

+#   dpcpp -fintelfpga -Xshardware -Xsboard=<FPGA_BOARD> -fsycl-link=early task_sequence.cpp -o task_sequence_report.a
+set(FPGA_EARLY_IMAGE ${TARGET_NAME}_report.a)
+# The compile output is not an executable, but an intermediate compilation result unique to DPC++.
+add_executable(${FPGA_EARLY_IMAGE} ${SOURCE_FILE})


Suggested change

add_executable(${FPGA_EARLY_IMAGE} ${SOURCE_FILE})

add_executable(${FPGA_EARLY_IMAGE} ${SOURCE_FILE})

target_include_directories(${FPGA_EARLY_IMAGE} PRIVATE ../../../../../include)

yuguen · 2022-10-21T09:23:11Z

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/src/CMakeLists.txt

+# CMake executes:
+#   [compile] dpcpp -fintelfpga -o task_sequence.cpp.o -c task_sequence.cpp
+#   [link]    dpcpp -fintelfpga -Xshardware -Xsboard=<FPGA_BOARD> task_sequence.cpp.o -o task_sequence.fpga
+add_executable(${FPGA_TARGET} EXCLUDE_FROM_ALL ${SOURCE_FILE})


Suggested change

add_executable(${FPGA_TARGET} EXCLUDE_FROM_ALL ${SOURCE_FILE})

add_executable(${FPGA_TARGET} EXCLUDE_FROM_ALL ${SOURCE_FILE})

target_include_directories(${FPGA_TARGET} PRIVATE ../../../../../include)

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/src/task_sequence.cpp

DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/third-party-programs.txt

…MakeLists.txt Co-authored-by: yuguen-intel <yohann.uguen@intel.com>

…ample.json Co-authored-by: yuguen-intel <yohann.uguen@intel.com>

…EADME.md Co-authored-by: yuguen-intel <yohann.uguen@intel.com>

…MakeLists.txt Co-authored-by: yuguen-intel <yohann.uguen@intel.com>

Co-authored-by: yuguen-intel <yohann.uguen@intel.com>

horobert and others added 2 commits July 18, 2022 15:30

Initial task_sequence code sample

9b5919c

Signed-off-by: horobert <horobert@intel.com>

Merge branch 'oneapi-src:master' into tasksequence

7b75ff3

rho180 requested a review from yuguen July 18, 2022 19:34

yuguen requested changes Jul 19, 2022

View reviewed changes

yuguen requested changes Oct 21, 2022

View reviewed changes

rho180 and others added 10 commits October 26, 2022 09:53

Update DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/C…

539c1ce

…MakeLists.txt Co-authored-by: yuguen-intel <yohann.uguen@intel.com>

Update DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/s…

f57c6f0

…ample.json Co-authored-by: yuguen-intel <yohann.uguen@intel.com>

Update DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/R…

4970367

…EADME.md Co-authored-by: yuguen-intel <yohann.uguen@intel.com>

Update DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/C…

0acaf3a

…MakeLists.txt Co-authored-by: yuguen-intel <yohann.uguen@intel.com>

Update DirectProgramming/DPC++FPGA/Tutorials/Features/task_sequence/C…

1bf4ecd

…MakeLists.txt Co-authored-by: yuguen-intel <yohann.uguen@intel.com>

Delete third-party-programs.txt

cfc2d7b

Delete task_sequence.vcxproj

701f8c2

Delete License.txt

cec2b24

Apply suggestions from code review

aa5c241

Co-authored-by: yuguen-intel <yohann.uguen@intel.com>

Delete task_sequence.sln

d3db081

	add_executable(${FPGA_EARLY_IMAGE} ${SOURCE_FILE})
	add_executable(${FPGA_EARLY_IMAGE} ${SOURCE_FILE})
	target_include_directories(${FPGA_EARLY_IMAGE} PRIVATE ../../../../../include)

	add_executable(${FPGA_TARGET} EXCLUDE_FROM_ALL ${SOURCE_FILE})
	add_executable(${FPGA_TARGET} EXCLUDE_FROM_ALL ${SOURCE_FILE})
	target_include_directories(${FPGA_TARGET} PRIVATE ../../../../../include)

Conversation

rho180 commented Jul 18, 2022

Existing Sample Changes

Description

External Dependencies

Type of change

How Has This Been Tested?

Adding a New Sample(s)

Description

Checklist

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yuguen Jul 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuguen commented Jul 19, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yuguen Jul 19, 2022 •

edited

Loading