Skip to content
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

[SYCL][Graph] Verbose E2E error messages #336

Closed
wants to merge 43 commits into from
Closed

Conversation

EwanC
Copy link
Collaborator

@EwanC EwanC commented Oct 24, 2023

The current error output from Graph E2E tests isn't particularly informative on the data that caused an assert to fail verification. As asserts are done on the whole container, rather than individual values. This change verifies each value individually so that a more informative error message can be printed when the assert fails. This is helpful for debugging issues that appear in CI but are hard to reproduce locally.

fabiomestre and others added 12 commits October 23, 2023 15:11
)

- Deprecate `read_image` for mipmap access and introduce `read_mipmap`
as the supported way to sample a mipmap
- Clarify this in the spec
- Note in the spec that using the wrong read with an image type will
result in undefined behaviour
There have been some problems with the merge from llvm.org around the
call to buildFatLOTDefaultPipeline(). I variable that got deleted
upstream was renamed and kept here. This change is intended to get
things back in sync.
…l#11520)

library_loading.cpp test is dependent on the ordering in which libraries
are loaded, causing failures in some builds. This commit addresses that
dependency by running FileCheck on each loaded library separately.
…sions of block_load/block_store, gather/scatter API (intel#11145)
…0975)

The `sycl::context` does not map clearly to a native context for CUDA
and HIP backends. This is especially true now that we are adding support
for multi device context intel#10737 . It
would be good to start this deprecation process. PRs to oneMKL and
oneDNN to follow
…ntel#11034)

Add support for -list, -check-section, and -unbundle for bundled BC
files in archives.

---------

Signed-off-by: Lu, John <john.lu@intel.com>
Rather issue an immediate diagnostic about the use of SEH, emit them
deferred similar to what is done for C++ exceptions.
…l#11595)

List default owner for sycl/e2e-tests and assign specific owners when
they are known (initial version based off of CODEOWNERS from
sycl-e2e-test repo)
As both `sycl::queue` and `sycl::handler` have similar `parallel_for`
member functions, it's easy to confuse the two when writing a command
group function. This change adds an exception with a clear error message
when `queue::submit` is called from a command group function.

---------

Signed-off-by: Michael Aziz <michael.aziz@intel.com>
@EwanC EwanC added the Graph Implementation Related to DPC++ implementation and testing label Oct 24, 2023
Copy link
Collaborator

@julianmi julianmi left a comment

Choose a reason for hiding this comment

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

Thanks, @EwanC. I think this is very valuable.

sycl/test-e2e/Graph/Inputs/add_nodes_after_finalize.cpp Outdated Show resolved Hide resolved
…pes (intel#11321)

Implements missing math builtins for the x86 target in libclc, only for
some scalar data types, missing types will be added in a follow up PR.
The builtins are defined as LLVM-IR in order to allow us to handle ABI
and directly call LLVM intrinsics.
Copy link
Collaborator

@mfrancepillois mfrancepillois left a comment

Choose a reason for hiding this comment

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

LGTM

sycl/test-e2e/Graph/Inputs/buffer_copy_2d.cpp Outdated Show resolved Hide resolved
sycl/test-e2e/Graph/Inputs/buffer_copy_host2target_2d.cpp Outdated Show resolved Hide resolved
asudarsa and others added 8 commits October 24, 2023 09:21
This PR fixes an issue that causes extra copy.

Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
- Store graph nodes inside a vector for more optimal searches
- Replace several depth first search operations with iterations over
node storage
- Node successors are now weak_ptrs
- Unit tests updated to reflect changes
This patch adds AST tests which we did not have before for the FPGA Loop
Attributes below:
1. [[intel::max_interleaving()]]
2.  [[intel::loop_coalesce]]
3. [[intel::max_concurrency()]]
4. [[intel::initiation_interval()]],
5. [[intel::speculated_iterations()]].

---------

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
Bumps [urllib3](https://github.com/urllib3/urllib3) from 2.0.6 to 2.0.7.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/urllib3/urllib3/releases">urllib3's
releases</a>.</em></p>
<blockquote>
<h2>2.0.7</h2>
<ul>
<li>Made body stripped from HTTP requests changing the request method to
GET after HTTP 303 &quot;See Other&quot; redirect responses.
(GHSA-g4mx-q9vg-27p4)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/urllib3/urllib3/blob/main/CHANGES.rst">urllib3's
changelog</a>.</em></p>
<blockquote>
<h1>2.0.7 (2023-10-17)</h1>
<ul>
<li>Made body stripped from HTTP requests changing the request method to
GET after HTTP 303 &quot;See Other&quot; redirect responses.</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/urllib3/urllib3/commit/56f01e088dc006c03d4ee6ea9da4ab810f1ed700"><code>56f01e0</code></a>
Release 2.0.7</li>
<li><a
href="https://github.com/urllib3/urllib3/commit/4e50fbc5db74e32cabd5ccc1ab81fc103adfe0b3"><code>4e50fbc</code></a>
Merge pull request from GHSA-g4mx-q9vg-27p4</li>
<li><a
href="https://github.com/urllib3/urllib3/commit/80808b04bfa68fbd099828848c96ee25df185f1d"><code>80808b0</code></a>
Fix docs build on Python 3.12 (<a
href="https://redirect.github.com/urllib3/urllib3/issues/3144">#3144</a>)</li>
<li><a
href="https://github.com/urllib3/urllib3/commit/f28deff1cf162c673b50d88d3552e91bda6d68a8"><code>f28deff</code></a>
Add 1.26.17 to the current changelog</li>
<li>See full diff in <a
href="https://github.com/urllib3/urllib3/compare/2.0.6...2.0.7">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=urllib3&package-manager=pip&previous-version=2.0.6&new-version=2.0.7)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the
[Security Alerts page](https://github.com/intel/llvm/network/alerts).

</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
As far as the compiler is concerned, the variable `x` is uninitialized
on some control-flow paths and is passed to the function
`(ex|in)clusive_scan_over_group`.

In terms of LLVM, the function parameter that `x` is passed to is likely
marked `noundef`. This can lead the compiler to take advantage of
`undef` rules and assume that the control path in which `x` is defined
must happen (otherwise it'd be undefined), and it can essentially remove
the guarding `i < N` around the memory accesses. This is, of course,
dangerous.

Though, in practice, this is often avoided because the scan function is
inlined before LLVM makes that assumption, this should not be relied
upon. I've chosen to zero-initialize it. The value of `init` cannot be
used because it may be out of bounds for the type pointed to by `InPtr`,
which can introduce new UB into the program.
Co-authored-by: Kenneth Benzie (Benie) <k.benzie83@gmail.com>
…l#11130)

Adds support for enabling the ability to perform a full device
compilation and link. Use the -fno-sycl-rdc option when also compiling
to object with -c. This will trigger the additional device linking steps
to be performed after the device compilation is completed.

Currently only supported for AOT enabled targets for spir64.

Upon consumption of these new fat objects, the driver will scan for
these unique binaries and instead of going through the device link,
these binaries will be sent directly to the host link step to be added
to the final executable. This is done by introducing a few triple
architecture values to designate target images in the fat objects
(spir64_gen_image, spir64_fpga_image, spir64_x86_64_image)

Performs some refactoring of the device link code, allowing for a common
platform for compile and link to access.
The static verifier reported a missing user defined copy constructor as
delete to match to the existing assignment operator.
v-klochkov and others added 22 commits October 24, 2023 14:47
…intel#11638)

- Added a routine to convert atomic_op to internal integer opcode that
might be different depending on the data type;
- Simplified calls of get_num_args() by removing some redundant LSC <->
non-LSC opcode conversions;
- Use fcmpxchg instead of obsolete/less-common fcmpwr alias-sets;
- Outlined FP types check for atomics into a macro
__ESIMD_FP_ATOMIC_OP_TYPE_CHECK that may be re-defined if necessary;
- NFC changes in lsc/atomic_smoke.cpp test to avoid potential problems
with some types that may not have all required constexpr constructors.

---------

Signed-off-by: Vyacheslav N Klochkov <vyacheslav.n.klochkov@intel.com>
That file is processed line-by-line, so `sycl/test-e2e` generic match by
the end of it rewrites previous decisions. Don't do it.

Also, move plugin/esimd/invoke_simd e2e tests ownership to corresponding
sections.
intel#11652)

This fixes loss of optnone attribute when generating spirv in O0 mode.
Co-authored-by: Michael Toguchi <michael.d.toguchi@intel.com>
…l#11647)

This is supposed to be allowed.

Uniform struct return already worked actually, I just reworded the error
and function.

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>
This commit adds the -fpreview-breaking-changes option to supersede the
SYCL2020_CONFORMANT_APIS preprocessor macro. This new option allows the
SYCL library to also make breaking changes outside a SYCL major release
by guarding breaking changes behind the __INTEL_PREVIEW_BREAKING_CHANGES
macro. When
-fpreview-breaking-changes is used together with -fsycl the compiled
program is linked against a variant of the SYCL library with the
__INTEL_PREVIEW_BREAKING_CHANGES macro defined.

---------

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
The patch add support for HIP via the libamd_comgr:

- Add build path in sycl-fusion
- Add finalization routine inside hip adapter's build program

---------

Signed-off-by: Victor Lomuller <victor@codeplay.com>
Co-authored-by: Lukas Sommer <lukas.sommer@codeplay.com>
Easier to copy-paste this way, since many CMake flags contain
semicolons.

Could be simplified by using shlex.join when Python 3.8 is required.
Changes:
* Added default constructor to `fpga_mem` and `fpga_datapath` in order
to avoid initialing the underlying data in the objects when they are
instantiated in function scope. This allows the compiler to optimize in
more cases as it avoids creating am additional write to the object. More
over it aligns the initialization behavior of a a type being
instantiated at function scope without `fpga_mem`/`fpga_datapath` and
with. They are now the same.
* Added tests to check for proper initialization behavior.
* Documented default constructors in SYCL specs
* Documented copy and move constructors and assignment operators
* Updated the nesting behavior of `fpga_mem` and `fpga_datapath` to
align it with what is currently supported through annotations.

---------

Co-authored-by: Steffen Larsen <steffen.larsen@intel.com>
Co-authored-by: Joe Garvey <joseph.garvey@intel.com>
…1663)

esimd emulator plugin was removed with
intel#11285. We can remove it from
CODEOWNERS
…st joint_matrix_load&joint_matrix_store for colC (intel#11604)
…1117)

Adding arithmetic overloaded operators for annotated_arg.

The motivation for adding these is to improve user experience when
templating `annotated_arg` with other templated classes that support
arithmetic operators. Example, without this change. the following SYCL
code results in errors:
```c++
// Some class that has arithmetic operators as member functions.
template<int W, bool S>
class ac_int;

struct MyKernel {
annotated_arg<ac_int<5, true>, ...> arg_a;
annotated_arg<ac_int<5, true>, ...> arg_b;
annotated_arg<ac_int<5, true>*, ...> arg_c;
void operator()() const {
  *arg_c = arg_a + arg_b; // error!
}
};
```
After this change, the above code will not result in errors as the new
operators will kick in and invoke the add operator from `ac_int`.
Alternatively, we considered what if we direct users to create free
floating operators for such cases. Eg, if we had:
```c++
template<...>
...   operator+(ac_int<...> a, ac_int<...> b) { ... }
```
This style still doesn't despite `annotated_arg` having an implicit
conversion operator (due to some C++ rules about implicit conversion I
think):
https://godbolt.org/z/h5cTTr17K

We are adding these generic operators here so that users do not have to
create these operators themselves.
Fix some leaks and a bug inside e2e tests.
Current instrumentation in SYCL runtime has additional overheads due to
argument capture even it is not subscribed to or needed. This patch
addresses this issue.

Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
…_matrix shape combinations (intel#11649)

Note that I did not touch the file in XMX8 as optimal combinations on
DG2 are different. So it is better to address DG2 case as a separate
task
Certain kernel fusion test cases fail when run using a compiler that's
not configured with the `--cuda` flag. This change disables these test
cases when there's no CUDA support.

Signed-off-by: Michael Aziz <michael.aziz@intel.com>
…r version of block_load/block_store, gather/scatter API (intel#11653)
The current error output from Graph E2E tests isn't
particularly informative on the data that caused an
assert to fail verification. As asserts are done on the
whole container, rather than individual values. This
change verifies each value individually so that a
more informative error message can be printed when
the assert fails. This is helpful for debugging
issues that appear in CI but are hard to reproduce locally.

Co-authored-by: Maxime France-Pillois <maxime.francepillois@codeplay.com>
@EwanC
Copy link
Collaborator Author

EwanC commented Oct 26, 2023

Moved to upstream PR intel#11666

@EwanC EwanC closed this Oct 26, 2023
@github-actions
Copy link

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:
darker --check --diff -r 8fe166fb4431645b6c7a26f4d88c0564e8d1d427..d65e8d90dde36ea07f6f38863e18536880ed99f0 buildbot/configure.py sycl/test-e2e/lit.cfg.py sycl/test/lit.cfg.py
View the diff from darker here.
--- buildbot/configure.py	2023-10-25 18:36:13.000000 +0000
+++ buildbot/configure.py	2023-10-26 08:31:11.890049 +0000
@@ -50,11 +50,11 @@
     llvm_enable_doxygen = 'OFF'
     llvm_enable_sphinx = 'OFF'
     llvm_build_shared_libs = 'OFF'
     llvm_enable_lld = 'OFF'
     sycl_enabled_plugins = ["opencl"]
-    sycl_preview_lib = 'ON'
+    sycl_preview_lib = "ON"
 
     sycl_enable_xpti_tracing = 'ON'
     xpti_enable_werror = 'OFF'
 
     if sys.platform != "darwin":
@@ -142,11 +142,11 @@
 
     if args.enable_plugin:
         sycl_enabled_plugins += args.enable_plugin
 
     if args.disable_preview_lib:
-        sycl_preview_lib = 'OFF'
+        sycl_preview_lib = "OFF"
 
     install_dir = os.path.join(abs_obj_dir, "install")
 
     cmake_cmd = [
         "cmake",
@@ -253,20 +253,47 @@
     parser.add_argument("--docs", action='store_true', help="build Doxygen documentation")
     parser.add_argument("--werror", action='store_true', help="Treat warnings as errors")
     parser.add_argument("--shared-libs", action='store_true', help="Build shared libraries")
     parser.add_argument("--cmake-opt", action='append', help="Additional CMake option not configured via script parameters")
     parser.add_argument("--cmake-gen", default="Ninja", help="CMake generator")
-    parser.add_argument("--use-libcxx", action="store_true", help="build sycl runtime with libcxx")
-    parser.add_argument("--libcxx-include", metavar="LIBCXX_INCLUDE_PATH", help="libcxx include path")
-    parser.add_argument("--libcxx-library", metavar="LIBCXX_LIBRARY_PATH", help="libcxx library path")
-    parser.add_argument("--use-lld", action="store_true", help="Use LLD linker for build")
-    parser.add_argument("--llvm-external-projects", help="Add external projects to build. Add as comma seperated list.")
-    parser.add_argument("--ci-defaults", action="store_true", help="Enable default CI parameters")
-    parser.add_argument("--enable-plugin", action='append', help="Enable SYCL plugin")
-    parser.add_argument("--disable-preview-lib", action='store_true', help="Disable building of the SYCL runtime major release preview library")
-    parser.add_argument("--disable-fusion", action="store_true", help="Disable the kernel fusion JIT compiler")
-    parser.add_argument("--add_security_flags", type=str, choices=['none', 'default', 'sanitize'], default=None, help="Enables security flags for compile & link. Two values are supported: 'default' and 'sanitize'. 'Sanitize' option is an extension of 'default' set.")
+    parser.add_argument(
+        "--use-libcxx", action="store_true", help="build sycl runtime with libcxx"
+    )
+    parser.add_argument(
+        "--libcxx-include", metavar="LIBCXX_INCLUDE_PATH", help="libcxx include path"
+    )
+    parser.add_argument(
+        "--libcxx-library", metavar="LIBCXX_LIBRARY_PATH", help="libcxx library path"
+    )
+    parser.add_argument(
+        "--use-lld", action="store_true", help="Use LLD linker for build"
+    )
+    parser.add_argument(
+        "--llvm-external-projects",
+        help="Add external projects to build. Add as comma seperated list.",
+    )
+    parser.add_argument(
+        "--ci-defaults", action="store_true", help="Enable default CI parameters"
+    )
+    parser.add_argument("--enable-plugin", action="append", help="Enable SYCL plugin")
+    parser.add_argument(
+        "--disable-preview-lib",
+        action="store_true",
+        help="Disable building of the SYCL runtime major release preview library",
+    )
+    parser.add_argument(
+        "--disable-fusion",
+        action="store_true",
+        help="Disable the kernel fusion JIT compiler",
+    )
+    parser.add_argument(
+        "--add_security_flags",
+        type=str,
+        choices=["none", "default", "sanitize"],
+        default=None,
+        help="Enables security flags for compile & link. Two values are supported: 'default' and 'sanitize'. 'Sanitize' option is an extension of 'default' set.",
+    )
     args = parser.parse_args()
 
     print("args:{}".format(args))
 
     return do_configure(args)
--- sycl/test/lit.cfg.py	2023-10-25 16:32:20.000000 +0000
+++ sycl/test/lit.cfg.py	2023-10-26 08:31:12.008161 +0000
@@ -59,11 +59,11 @@
            lit_config.note("\tUnset "+var)
            llvm_config.with_environment(var,"")
 
 # If major release preview library is enabled we can enable the feature.
 if config.sycl_preview_lib_enabled == "ON":
-    config.available_features.add('preview-breaking-changes-supported')
+    config.available_features.add("preview-breaking-changes-supported")
 
 # Configure LD_LIBRARY_PATH or corresponding os-specific alternatives
 # Add 'libcxx' feature to filter out all SYCL abi tests when SYCL runtime
 # is built with llvm libcxx. This feature is added for Linux only since MSVC
 # CL compiler doesn't support to use llvm libcxx instead of MSVC STL.
--- sycl/test-e2e/lit.cfg.py	2023-10-25 16:32:20.000000 +0000
+++ sycl/test-e2e/lit.cfg.py	2023-10-26 08:31:12.290110 +0000
@@ -164,21 +164,25 @@
     config.substitutions.append( ('%level_zero_options', level_zero_options) )
 else:
     config.substitutions.append( ('%level_zero_options', '') )
 
 # Check for sycl-preview library
-check_preview_breaking_changes_file='preview_breaking_changes_link.cpp'
-with open(check_preview_breaking_changes_file, 'w') as fp:
-    fp.write('#include <sycl/sycl.hpp>')
-    fp.write('namespace sycl { inline namespace _V1 { namespace detail {')
-    fp.write('extern void PreviewMajorReleaseMarker();')
-    fp.write('}}}')
-    fp.write('int main() { sycl::detail::PreviewMajorReleaseMarker(); return 0; }')
-
-sp = subprocess.getstatusoutput(config.dpcpp_compiler+' -fsycl -fpreview-breaking-changes ' + check_preview_breaking_changes_file)
+check_preview_breaking_changes_file = "preview_breaking_changes_link.cpp"
+with open(check_preview_breaking_changes_file, "w") as fp:
+    fp.write("#include <sycl/sycl.hpp>")
+    fp.write("namespace sycl { inline namespace _V1 { namespace detail {")
+    fp.write("extern void PreviewMajorReleaseMarker();")
+    fp.write("}}}")
+    fp.write("int main() { sycl::detail::PreviewMajorReleaseMarker(); return 0; }")
+
+sp = subprocess.getstatusoutput(
+    config.dpcpp_compiler
+    + " -fsycl -fpreview-breaking-changes "
+    + check_preview_breaking_changes_file
+)
 if sp[0] == 0:
-    config.available_features.add('preview-breaking-changes-supported')
+    config.available_features.add("preview-breaking-changes-supported")
 
 # Check for CUDA SDK
 check_cuda_file='cuda_include.cpp'
 with open(check_cuda_file, 'w') as fp:
     fp.write('#include <cuda.h>\n')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graph Implementation Related to DPC++ implementation and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.