[SYCL] Added support of rounding modes for non-host devices by fadeeval · Pull Request #1463 · intel/llvm

fadeeval · 2020-04-02T17:55:17Z

Implementing rounding models for cl::sycl::vec type for non host devices.

Signed-off-by: Aleksander Fadeev aleksander.fadeev@intel.com

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

AlexeySachkov · 2020-04-02T19:18:07Z

Tagging @Ruyk to take a look at changes at SPIRVBuiltins.td

AlexeySachkov · 2020-04-02T19:18:27Z

sycl/include/CL/sycl/types.hpp

+template <typename T, typename R, rounding_mode roundingMode>
+detail::enable_if_t<!std::is_same<T, R>::value &&
+                        (is_int_to_int<T, R>::value &&
+                         !(is_int_to_from_uint<T, R>::value) ||


This line seem cause build error, please double-check

I forgot to delete it.

sycl/test/basic_tests/vec_convert.cpp

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Ruyk · 2020-04-10T18:39:24Z

This connects to @Naghasan 's work

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Naghasan · 2020-04-14T08:58:50Z

clang/lib/Sema/SPIRVBuiltins.td

 }

-foreach IType = [Char, Short, Int, Long] in {
+foreach IType = [TrueChar, Short, Int, Long] in {


changing https://github.com/intel/llvm/pull/1463/files#diff-35388f048c1dc54db0a9491da9317c69R290 to char is the actual fix I believe. This change will trigger unwanted implicit conversions.

Char is bound to std::int8_t (always signed char) while TrueChar is the actual char type which can be unsigned. I added TrueChar as a hack for the __spirv_ocl_printf but I don't think it should be kept.

Sorry the link don't seem to work properly, this line

llvm/clang/lib/Sema/SPIRVBuiltins.td

Line 290 in e875fbf

def Char : IntType<"schar", QualType<"SignedCharTy", 0, 1>, 8>;

needs to changed from schar to char so the function name can be built correctly (with e875fbf it is now post-fixing with _Rschar)

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

sycl/test/basic_tests/buffer/buffer_dev_to_dev.cpp

sycl/test/basic_tests/vec_convert.cpp

bader · 2020-04-15T10:30:05Z

sycl/test/basic_tests/vec_convert.cpp

      half8{+2.3f, +2.5f, +2.7f, -2.3f, -2.5f, -2.7f, 0.f, 0.f});
-
  return 0;
-}
+}


Don't see much value in these changes.
Suggest keeping original code.

Only part of this comment was resolved.

I don't understand, why they are different?

You seem to delete line ending character.

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

bader · 2020-04-15T15:13:25Z

sycl/test/basic_tests/buffer/buffer_dev_to_dev.cpp

+// Unsupported: cuda
+//  The test fails sporadically on cuda. See https://github.com/intel/llvm/issues/1508 for more details.
+


Please, revert changes in this file.
d9b2d52 already disabled the test.

Do I need to do a rebase?

No. CI system does the merge with tip of the sycl branch before testing.

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

bader

LGTM.
The description doesn't say this, but this patch implements conversion from FP to Int only. What about Int to FP?

fadeeval · 2020-04-16T11:02:13Z

LGTM.
The description doesn't say this, but this patch implements conversion from FP to Int only. What about Int to FP?

I implemented this in the other PR.

bader · 2020-04-22T11:06:17Z

@Naghasan, @erichkeane, @Fznamznon, @turinevgeny, please approve if you don't have any other comments.

turinevgeny · 2020-04-22T11:33:47Z

LGTM.

bader · 2020-04-22T11:37:13Z

@turinevgeny, you must approve using GitHub process: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/approving-a-pull-request-with-required-reviews.

turinevgeny

LGTM.

…versioning * origin/sycl: [XPTI][Framework] Reference implementation of the Xpti framework to be used with instrumentation in SYCL (intel#1557) [SYCL] Initial ABI checks implementation (intel#1528) [SYCL] Support connection with multiple plugins (intel#1490) [SYCL] Add a new header file with the reduction class definition (intel#1558) [SYCL] Add test for SYCL kernels with accessor and spec constant (intel#1536) [SYCL][CUDA] Move interop tests (intel#1570) [Driver][SYCL] Remove COFF object format designator for Windows device compiles (intel#1574) [SYCL] Fix conflicting visibility attributes (intel#1571) [SYCL][DOC] Update the SYCL Runtime Interface document with design details (intel#680) [SYCL] Improve image accessors support on a host device (intel#1502) [SYCL] Make queue's non-USM event ownership temporary (intel#1561) [SYCL] Added support of rounding modes for non-host devices (intel#1463) [SYCL] SemaSYCL significant refactoring (intel#1517) [SYCL] Support 0-dim accessor in handler::copy(accessor, accessor) (intel#1551)

fadeeval added 18 commits March 27, 2020 16:57

support_rounding_models_for_non-host_devices intel#1.1

916dba8

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

support_rounding_models_for_non-host_devices intel#1.2

0dba70d

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

support_rounding_models_for_non-host_devices intel#1.3

0443893

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Raw commit

cf7c3d4

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Alpha version

b4a9f93

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Alppha 2

c671d9b

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Origin

6c43eca

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Operative commit

bb88eff

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Fix the return type

cf8e720

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Converting SYCL types in OpenCL types

bbad4df

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Formatting and assert changes

b9a171e

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Fix of the spirv functions declaration

f927a3e

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Metaprogramming

2a96e9b

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Adding macros

a03b462

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Restore spirv_ops.hpp

070fbf5

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Macros improving

9c13847

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Unsign to sign convert

543fe09

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Reverting

ec8806a

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

fadeeval requested a review from AlexeySachkov April 2, 2020 17:55

AlexeySachkov requested a review from garimagu April 2, 2020 19:16

AlexeySachkov assigned alexbatashev and KarachunIvan Apr 2, 2020

AlexeySachkov reviewed Apr 2, 2020

View reviewed changes

Patch

870059e

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

fadeeval requested a review from bader as a code owner April 3, 2020 07:12

fadeeval added 3 commits April 3, 2020 10:17

Formatting

ea768af

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Formatting 2

238ee1a

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Revert

c03698b

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

fadeeval requested a review from erichkeane as a code owner April 3, 2020 08:26

fadeeval added 3 commits April 13, 2020 13:39

delete unsupported: cuda

2a1fb94

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

vec_convert.cpp XFail: cuda

8921bc3

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Unsupported on cuda buffer_dev_to_dev.cpp

f6d6777

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Naghasan suggested changes Apr 14, 2020

View reviewed changes

fadeeval added 2 commits April 14, 2020 13:11

TrueChar to Char

e875fbf

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

schar to char

f1f2897

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Naghasan mentioned this pull request Apr 14, 2020

[SPIR-V] Correct/improve declaration of some SPIR-V builtins #1519

Merged

fix TrueChar

c417b51

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

bader reviewed Apr 15, 2020

View reviewed changes

sycl/test/basic_tests/buffer/buffer_dev_to_dev.cpp Outdated Show resolved Hide resolved

sycl/test/basic_tests/vec_convert.cpp Outdated Show resolved Hide resolved

bader reviewed Apr 15, 2020

View reviewed changes

Comments fix

014c0d4

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

fadeeval requested review from AlexeySachkov, bader and romanovvlad April 15, 2020 15:05

bader requested changes Apr 15, 2020

View reviewed changes

Fix

b0eb8e0

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

fadeeval requested a review from bader April 16, 2020 07:03

bader approved these changes Apr 16, 2020

View reviewed changes

romanovvlad approved these changes Apr 21, 2020

View reviewed changes

turinevgeny approved these changes Apr 22, 2020

View reviewed changes

Fznamznon approved these changes Apr 22, 2020

View reviewed changes

bader merged commit 7e3cca4 into intel:sycl Apr 22, 2020

fadeeval deleted the private/fadeeval/support_rounding_models_for_non-host_devices branch May 21, 2020 08:23

		// Unsupported: cuda
		// The test fails sporadically on cuda. See https://github.com/intel/llvm/issues/1508 for more details.

Comments

Conversation

fadeeval commented Apr 2, 2020

Uh oh!

AlexeySachkov commented Apr 2, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Ruyk commented Apr 10, 2020

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!

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!

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!

bader left a comment

Choose a reason for hiding this comment

Uh oh!

fadeeval commented Apr 16, 2020

Uh oh!

bader commented Apr 22, 2020

Uh oh!

turinevgeny commented Apr 22, 2020

Uh oh!

bader commented Apr 22, 2020

Uh oh!

turinevgeny left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants