[ESIMD] Implement stateless memory accesses enforcement #6287

v-klochkov · 2022-06-10T06:30:31Z

The driver option -f[no-]sycl-esimd-force-stateless-mem is added.

-fsycl-esimd-force-stateless-mem enables the automatic conversion of stateful memory
accesses via SYCL accessors or surface-index to stateless within ESIMD kernels.
It also disables those ESIMD intrinsics that use stateful accesses that cannot be converted to stateless.
-fsycl-esimd-force-stateless-mem defines the macro __ESIMD_FORCE_STATELESS_MEM
to map the calls of ESIMD API using accessors to calls of API using pointers.
It also passes a switch to sycl-post-link to signal it that it should
ignore the buffer_t attribute and use svmptr_t.

-fno-sycl-esimd-force-stateless-mem is used to tell the compiler not to convert
stateful memory accesses to stateless. Default behavior.

Draft of the design document/proposal for this change-set: #6187

The driver option -fsycl-esimd-stateless-access makes the compiler to use stateless memory accesses for those ESIMD kernels and ESIMD intrinsics that use accessors. The option (if used) defines the macro ESIMD_FORCE_STATELESS_MEM_ACCESS to map the calls of API using accessors to calls of API using USM pointers. It also passes a switch to sycl-post-link to signal it that it should ignore the buffer_t attribute and use svmptr_t. Signed-off-by: Vyacheslav N Klochkov <[email protected]>

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

clang/include/clang/Basic/LangOptions.def

clang/include/clang/Driver/Options.td

clang/lib/Frontend/InitPreprocessor.cpp

llvm/include/llvm/SYCLLowerIR/ESIMD/ESIMDVerifier.h

sycl/include/sycl/ext/intel/esimd/memory.hpp

sycl/include/sycl/ext/intel/experimental/esimd/memory.hpp

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

…ttern) Signed-off-by: Vyacheslav N Klochkov <[email protected]>

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

…S_MEM Some of intrinsics do not have USM equivalents for their accessros forms, i.e. get_surface_index, raw_send_load/store. Signed-off-by: Vyacheslav N Klochkov <[email protected]>

mdtoguchi · 2022-06-14T01:12:31Z

clang/include/clang/Driver/Options.td

+defm sycl_esimd_force_stateless_mem : BoolFOption<"sycl-esimd-force-stateless-mem",
+    LangOpts<"SYCLESIMDForceStatelessMem">, DefaultFalse,
+    PosFlag<SetTrue, [CC1Option, CoreOption], "Convert accesses via accessors to USM memory accesses in ESIMD kernels">,
+    NegFlag<SetFalse>>;


Should also be a CoreOption to be available on Windows command lines

Mike, Thank you for reviewing the PR. Can you explain what you mean, or maybe add a 'suggestion'?
Isn't it already a CoreOption (see the line 2793)?

The positive variant of the option is a CoreOption but the negative isn't. I don't think the Flags from the Pos side are also applied to the Neg side unless BothFlags is used to do so.

I added the BothFlags to address this comment. Thank you!

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

pvchupin · 2022-06-14T19:57:40Z

clang/include/clang/Driver/Options.td

@@ -2788,6 +2788,11 @@ def fintelfpga : Flag<["-"], "fintelfpga">, Group<f_Group>,
  Flags<[CC1Option, CoreOption]>, HelpText<"Perform ahead of time compilation for FPGA">;
 def fsycl_device_only : Flag<["-"], "fsycl-device-only">, Flags<[CoreOption]>,
  HelpText<"Compile SYCL kernels for device">;
+defm sycl_esimd_force_stateless_mem : BoolFOption<"sycl-esimd-force-stateless-mem",
+    LangOpts<"SYCLESIMDForceStatelessMem">, DefaultFalse,
+    PosFlag<SetTrue, [CC1Option, CoreOption], "Convert accesses via accessors to USM memory accesses in ESIMD kernels">,


It should be also marked experimental in help message. See other examples.

It is marked as experimental. Thanks.

pvchupin · 2022-06-14T19:58:50Z

sycl/doc/UsersManual.md

@@ -290,6 +290,28 @@ and not recommended to use in production environment.

    NOTE: This flag is currently only supported with the CUDA and HIP targets.

+
+**`-f[no]-sycl-esimd-force-stateless-mem`** [EXPERIMENTAL]


Suggested change

**`-f[no]-sycl-esimd-force-stateless-mem`** [EXPERIMENTAL]

**`-f[no-]sycl-esimd-force-stateless-mem`** [EXPERIMENTAL]

pvchupin · 2022-06-14T20:00:47Z

sycl/doc/UsersManual.md

+    "stateful" memory accesses via SYCL accessors to "stateless" accesses
+    within ESIMD (Explicit SIMD) kernels by compiler.
+
+    This option disables the intrinsics and methods accepting SYCL accessors


It disables in -f or -fno- form? Can you clarify here?

-f option disables those intrinsics that cannot be converted automatically. I updated the text. Thank you.

pvchupin · 2022-06-14T20:01:42Z

sycl/doc/UsersManual.md

+    does not have  the limit of 4Gb per surface.
+    Also, some of Intel GPUs or GPU run-time/drivers may support only
+    "stateless" memory accesses.
+


Please add information about default value.

Added. Thank you.

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

clang/test/Preprocessor/sycl-esimd-force-stateless-mem.cpp

sycl/doc/UsersManual.md

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

pvchupin · 2022-06-15T19:52:43Z

sycl/doc/UsersManual.md

+    The stateless memory access enforcement is disabled by default.
+
+    -fno-sycl-esimd-force-stateless-mem is used to tell compiler not to
+    enforce usage of stateless memory accesses.


Suggested change

The stateless memory access enforcement is disabled by default.

-fno-sycl-esimd-force-stateless-mem is used to tell compiler not to

enforce usage of stateless memory accesses.

-fno-sycl-esimd-force-stateless-mem is used to tell compiler not to

enforce usage of stateless memory accesses. This is default behavior.

nit: looks a bit clearer to me this way.

Ok, Thank you. Fixed here: 11d9c6d

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

againull

LGTM

premanandrao

FE changes look okay to me.

smanna12

FE changes LGTM

elizabethandrews

FE changes and doc LGTM. Thank you

v-klochkov · 2022-06-21T18:50:55Z

@hchilama - I had a comment from @mdtoguchi (#6287 (comment)), and I believe I addressed it in additional changes.
Would you please review the changes in driver, while Mike is on vacation?

v-klochkov added 2 commits June 9, 2022 21:31

Add the bool argument EnforceStateless to ESIMDVerifier pass

eadc644

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

v-klochkov requested a review from kbobrovs June 10, 2022 06:30

clang-format

1568f79

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

kbobrovs reviewed Jun 10, 2022

View reviewed changes

v-klochkov added 8 commits June 10, 2022 13:41

Address reviewer's comments (all except more tests and documentation)

cda2d69

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

Merge remote-tracking branch 'intel_llvm/sycl' into esimd_stateless_acc

225d5a1

Address reviewer's comments (couple tests + func for repeated code pa…

e6bb2e9

…ttern) Signed-off-by: Vyacheslav N Klochkov <[email protected]>

Merge remote-tracking branch 'intel_llvm/sycl' into esimd_stateless_acc

ceeed8d

Fix an error in scatter implementation

6034502

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

Merge remote-tracking branch 'intel_llvm/sycl' into esimd_stateless_acc

50e06cb

Fix LIT test

da4184f

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

Disable intrinsics that are not supported with __ESIMD_FORCE_STATELES…

d4383ad

…S_MEM Some of intrinsics do not have USM equivalents for their accessros forms, i.e. get_surface_index, raw_send_load/store. Signed-off-by: Vyacheslav N Klochkov <[email protected]>

v-klochkov marked this pull request as ready for review June 14, 2022 01:02

v-klochkov requested review from a team as code owners June 14, 2022 01:02

v-klochkov requested a review from againull June 14, 2022 01:03

mdtoguchi reviewed Jun 14, 2022

View reviewed changes

Add the description for -fsycl-esimd-force-stateless-mem to user manual

55b3c24

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

v-klochkov requested a review from a team as a code owner June 14, 2022 02:57

Marked the new option with the "EXPERIMENTAL" keyword

1f3f757

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

pvchupin reviewed Jun 14, 2022

View reviewed changes

v-klochkov added 2 commits June 14, 2022 13:30

Update the option description/definition in driver. Update UserManual.

8559274

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

Merge remote-tracking branch 'intel_llvm/sycl' into esimd_stateless_acc

80b009d

elizabethandrews reviewed Jun 15, 2022

View reviewed changes

clang/test/Preprocessor/sycl-esimd-force-stateless-mem.cpp Show resolved Hide resolved

sycl/doc/UsersManual.md Outdated Show resolved Hide resolved

sycl/doc/UsersManual.md Outdated Show resolved Hide resolved

sycl/doc/UsersManual.md Outdated Show resolved Hide resolved

v-klochkov added 2 commits June 15, 2022 11:43

Address reviewer's comments.

fc687f6

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

Merge remote-tracking branch 'intel_llvm/sycl' into esimd_stateless_acc

e910230

pvchupin reviewed Jun 15, 2022

View reviewed changes

Address reviewer's comment (fix in UserManual.md only)

11d9c6d

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

againull approved these changes Jun 15, 2022

View reviewed changes

premanandrao approved these changes Jun 16, 2022

View reviewed changes

smanna12 approved these changes Jun 16, 2022

View reviewed changes

elizabethandrews approved these changes Jun 16, 2022

View reviewed changes

pvchupin approved these changes Jun 21, 2022

View reviewed changes

pvchupin requested a review from mdtoguchi June 21, 2022 18:19

pvchupin mentioned this pull request Jun 21, 2022

[ESIMD] Stateful to stateless mem access conversion design. #6187

Merged

v-klochkov requested a review from hchilama June 22, 2022 02:52

hchilama approved these changes Jun 22, 2022

View reviewed changes

pvchupin merged commit 1811162 into intel:sycl Jun 22, 2022

v-klochkov deleted the esimd_stateless_acc branch June 22, 2022 17:57

		@@ -290,6 +290,28 @@ and not recommended to use in production environment.

		NOTE: This flag is currently only supported with the CUDA and HIP targets.


		`-f[no]-sycl-esimd-force-stateless-mem` [EXPERIMENTAL]

	`-f[no]-sycl-esimd-force-stateless-mem` [EXPERIMENTAL]
	`-f[no-]sycl-esimd-force-stateless-mem` [EXPERIMENTAL]

[ESIMD] Implement stateless memory accesses enforcement #6287

[ESIMD] Implement stateless memory accesses enforcement #6287

Uh oh!

Conversation

v-klochkov commented Jun 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

againull left a comment

Choose a reason for hiding this comment

Uh oh!

premanandrao left a comment

Choose a reason for hiding this comment

Uh oh!

smanna12 left a comment

Choose a reason for hiding this comment

Uh oh!

elizabethandrews left a comment

Choose a reason for hiding this comment

Uh oh!

v-klochkov commented Jun 21, 2022

Uh oh!

Uh oh!

v-klochkov commented Jun 10, 2022 •

edited

Loading