[XLA:GPU] Add SPIRV-LLVM-Translator and translation pass #11424

ShengYang1 · 2024-04-11T02:26:31Z

It is a sub PR of #9042 to add spirv-llvm-translator and translation pass

kamaljeeti · 2024-05-07T05:43:05Z

Hi @penpornk , could you please review this PR? Thanks.

penpornk

Thank you for the PR and sorry for the delay!

penpornk · 2024-05-07T11:55:47Z

third_party/tsl/third_party/spirv_llvm_translator/spirv_llvm_translator.patch

+-  SPIRAS_Private,
+  SPIRAS_Generic,
+   SPIRAS_Global,
+-  SPIRAS_Constant,
+  SPIRAS_Internal,
+   SPIRAS_Local,
+-  SPIRAS_Generic,
+  SPIRAS_Constant,
+  SPIRAS_Private,


Just curious, why do we need to reorder them?

Spir backend uses different addrspace representations link with nvptx backend link. We reorder the enum value here so that we can make xla llvm codegen simple(avoiding changing addrspace based on device backend everywhere)

This is useful context for people who are unfamiliar with the backend.

Could you please add this explanation in this patch file (between line 2 and 3)? I believe the patch tool will ignore anything above the --- a/lib/SPIRV/SPIRVInternal.h line. E.g.,

diff --git a/lib/SPIRV/SPIRVInternal.h b/lib/SPIRV/SPIRVInternal.h index a828add8..924e13b4 100644 <Explanation here> --- a/lib/SPIRV/SPIRVInternal.h

penpornk · 2024-05-07T12:04:02Z

xla/service/gpu/llvm_gpu_backend/gpu_backend_lib.cc

+  FeedLLVMWithFlags({"-slp-vectorize-hor=false"});
+
+  FeedLLVMWithFlags({
+      "-slp-min-reg-size=64",
+      "-slp-max-reg-size=64",
+  });


Can these be merged into one call?

Suggested change

FeedLLVMWithFlags({"-slp-vectorize-hor=false"});

FeedLLVMWithFlags({

"-slp-min-reg-size=64",

"-slp-max-reg-size=64",

});

FeedLLVMWithFlags({

"-slp-vectorize-hor=false"

"-slp-min-reg-size=64",

"-slp-max-reg-size=64",

});

penpornk · 2024-05-07T12:05:59Z

xla/service/gpu/llvm_gpu_backend/gpu_backend_lib.cc

@@ -1075,5 +1082,92 @@ absl::StatusOr<std::vector<uint8_t>> CompileToHsaco(

 }  // namespace amdgpu

+namespace {


Nit: Please add a blank line after this.

penpornk · 2024-05-07T12:06:31Z

xla/service/gpu/llvm_gpu_backend/gpu_backend_lib.cc

+  llvm::PassRegistry* registry = llvm::PassRegistry::getPassRegistry();
+  InitializePasses(registry);
+}
+}  // namespace


Nit: Please add a blank line before this.
Please do the same for namespace spir.

penpornk · 2024-05-08T04:31:15Z

third_party/tsl/third_party/spirv_llvm_translator/spirv_llvm_translator.patch

+-  SPIRAS_Private,
+  SPIRAS_Generic,
+   SPIRAS_Global,
+-  SPIRAS_Constant,
+  SPIRAS_Internal,
+   SPIRAS_Local,
+-  SPIRAS_Generic,
+  SPIRAS_Constant,
+  SPIRAS_Private,


This is useful context for people who are unfamiliar with the backend.

Could you please add this explanation in this patch file (between line 2 and 3)? I believe the patch tool will ignore anything above the --- a/lib/SPIRV/SPIRVInternal.h line. E.g.,

diff --git a/lib/SPIRV/SPIRVInternal.h b/lib/SPIRV/SPIRVInternal.h index a828add8..924e13b4 100644 <Explanation here> --- a/lib/SPIRV/SPIRVInternal.h

penpornk

Thank you very much!

NaiyerRizz · 2024-05-14T09:53:14Z

Hi @penpornk
This PR has been approved but not yet merged, Can you please look into this.

dimitar-asenov · 2024-05-16T09:56:20Z

@ShengYang1 Could you please rebase the PR on top of the latest state of the repo? Thanks!

ShengYang1 · 2024-05-17T06:53:40Z

@ShengYang1 Could you please rebase the PR on top of the latest state of the repo? Thanks!

Done.

penpornk

Thank you!

dimitar-asenov · 2024-05-17T11:27:02Z

@ddunl This fails to import internally because copybara isn't sure where to put the new spirv files. It seems like it requires some extra configuration. Could you please advise what's needed to merge this? Thanks.

akuegel · 2024-05-24T05:22:50Z

@ddunl This fails to import internally because copybara isn't sure where to put the new spirv files. It seems like it requires some extra configuration. Could you please advise what's needed to merge this? Thanks.

TSL repository cannot be changed via PRs, you need to copy the changes manually as all changes to TSL repo are not imported.

ShengYang1 · 2024-05-28T07:41:10Z

@ddunl This fails to import internally because copybara isn't sure where to put the new spirv files. It seems like it requires some extra configuration. Could you please advise what's needed to merge this? Thanks.

TSL repository cannot be changed via PRs, you need to copy the changes manually as all changes to TSL repo are not imported.

@akuegel Should I submit another PR for tsl changes?

akuegel · 2024-05-28T08:13:28Z

@ddunl This fails to import internally because copybara isn't sure where to put the new spirv files. It seems like it requires some extra configuration. Could you please advise what's needed to merge this? Thanks.

TSL repository cannot be changed via PRs, you need to copy the changes manually as all changes to TSL repo are not imported.

@akuegel Should I submit another PR for tsl changes?

No, unfortunately it can only be changed internally.

@xla-rotation importing this PR requires manual work, see comments above.

dimitar-asenov · 2024-07-10T08:40:35Z

This PR needs #11425 to be submitted first together with some internal changes. I'm working on those. Once that other PR is in, I will look into this one.

Imported from GitHub PR #11424 It is a sub PR of #9042 to add spirv-llvm-translator and translation pass Copybara import of the project: -- 6d8ce76 by Sheng, Yang <[email protected]>: Add SPIRV-LLVM-Translator and translation pass update comments and SPIRV-LLVM-Translator commit Merging this change closes #11424 FUTURE_COPYBARA_INTEGRATE_REVIEW=#11424 from Intel-tensorflow:yang/llvm-spirv 6d8ce76 PiperOrigin-RevId: 661176906

FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#11424 from Intel-tensorflow:yang/llvm-spirv 6d8ce76e2b71120106ffae91945df9b974e74dec PiperOrigin-RevId: 661065795

FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#11424 from Intel-tensorflow:yang/llvm-spirv 6d8ce76e2b71120106ffae91945df9b974e74dec PiperOrigin-RevId: 660362390

FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#11424 from Intel-tensorflow:yang/llvm-spirv 6d8ce76e2b71120106ffae91945df9b974e74dec PiperOrigin-RevId: 661192298

Imported from GitHub PR openxla/xla#11424 It is a sub PR of openxla/xla#9042 to add spirv-llvm-translator and translation pass Copybara import of the project: -- 6d8ce76e2b71120106ffae91945df9b974e74dec by Sheng, Yang <[email protected]>: Add SPIRV-LLVM-Translator and translation pass update comments and SPIRV-LLVM-Translator commit Merging this change closes #11424 PiperOrigin-RevId: 661186664

`protoc` has been warning of this unused import for a while now, so let's finally remove it to clean up build logs. FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#11424 from Intel-tensorflow:yang/llvm-spirv 6d8ce76e2b71120106ffae91945df9b974e74dec PiperOrigin-RevId: 661195599

FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#11424 from Intel-tensorflow:yang/llvm-spirv 6d8ce76e2b71120106ffae91945df9b974e74dec PiperOrigin-RevId: 660608962

FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#11424 from Intel-tensorflow:yang/llvm-spirv 6d8ce76e2b71120106ffae91945df9b974e74dec PiperOrigin-RevId: 661065783

golechwierowicz · 2024-08-09T11:59:35Z

This has not fully landed yet due to changes in tsl.

I'll let you know once this lands.

Imported from GitHub PR openxla/xla#11424 It is a sub PR of openxla/xla#9042 to add spirv-llvm-translator and translation pass Copybara import of the project: -- 6d8ce76e2b71120106ffae91945df9b974e74dec by Sheng, Yang <[email protected]>: Add SPIRV-LLVM-Translator and translation pass update comments and SPIRV-LLVM-Translator commit Merging this change closes #11424 FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#11424 from Intel-tensorflow:yang/llvm-spirv 6d8ce76e2b71120106ffae91945df9b974e74dec PiperOrigin-RevId: 661222386

golechwierowicz · 2024-08-09T15:33:19Z

This change should be in.

Imported from GitHub PR openxla/xla#11424 It is a sub PR of openxla/xla#9042 to add spirv-llvm-translator and translation pass Copybara import of the project: -- 6d8ce76e2b71120106ffae91945df9b974e74dec by Sheng, Yang <[email protected]>: Add SPIRV-LLVM-Translator and translation pass update comments and SPIRV-LLVM-Translator commit Merging this change closes #11424 PiperOrigin-RevId: 661272459

Imported from GitHub PR #11424 It is a sub PR of #9042 to add spirv-llvm-translator and translation pass Copybara import of the project: -- 6d8ce76 by Sheng, Yang <[email protected]>: Add SPIRV-LLVM-Translator and translation pass update comments and SPIRV-LLVM-Translator commit Merging this change closes #11424 COPYBARA_INTEGRATE_REVIEW=#11424 from Intel-tensorflow:yang/llvm-spirv 6d8ce76 PiperOrigin-RevId: 661272459

Imported from GitHub PR openxla/xla#11424 It is a sub PR of openxla/xla#9042 to add spirv-llvm-translator and translation pass Copybara import of the project: -- 6d8ce76e2b71120106ffae91945df9b974e74dec by Sheng, Yang <[email protected]>: Add SPIRV-LLVM-Translator and translation pass update comments and SPIRV-LLVM-Translator commit Merging this change closes #11424 PiperOrigin-RevId: 661272459

Some parts of it will likely be re-used in a different Dockerfile. FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#11424 from Intel-tensorflow:yang/llvm-spirv 6d8ce76e2b71120106ffae91945df9b974e74dec PiperOrigin-RevId: 661274311

Imported from GitHub PR #11424 It is a sub PR of #9042 to add spirv-llvm-translator and translation pass Copybara import of the project: -- 6d8ce76 by Sheng, Yang <[email protected]>: Add SPIRV-LLVM-Translator and translation pass update comments and SPIRV-LLVM-Translator commit Merging this change closes #11424 COPYBARA_INTEGRATE_REVIEW=#11424 from Intel-tensorflow:yang/llvm-spirv 6d8ce76 PiperOrigin-RevId: 661186664

Imported from GitHub PR #11424 It is a sub PR of #9042 to add spirv-llvm-translator and translation pass Copybara import of the project: -- 6d8ce76 by Sheng, Yang <[email protected]>: Add SPIRV-LLVM-Translator and translation pass update comments and SPIRV-LLVM-Translator commit Merging this change closes #11424 COPYBARA_INTEGRATE_REVIEW=#11424 from Intel-tensorflow:yang/llvm-spirv 6d8ce76 PiperOrigin-RevId: 661272459

Zantares · 2024-08-13T03:06:34Z

This change should be in.

We've been notified that the changes are merged and I have updated the internal PR track list, thank you very much!

github-actions bot added the kokoro:force-run Forces CI to rerun label Apr 11, 2024

github-actions bot assigned kamaljeeti and xla-rotation Apr 11, 2024

kokoro-team removed the kokoro:force-run Forces CI to rerun label Apr 11, 2024

Zantares mentioned this pull request Apr 11, 2024

[Draft][XLA:GPU] Add sycl runtime #9042

Draft

kamaljeeti requested a review from golechwierowicz April 12, 2024 05:32

golechwierowicz requested a review from penpornk April 12, 2024 09:34

penpornk suggested changes May 7, 2024

View reviewed changes

ShengYang1 force-pushed the yang/llvm-spirv branch from a1db193 to 9abfdb8 Compare May 8, 2024 01:32

github-actions bot added the kokoro:force-run Forces CI to rerun label May 8, 2024

kokoro-team removed the kokoro:force-run Forces CI to rerun label May 8, 2024

penpornk suggested changes May 8, 2024

View reviewed changes

github-actions bot added the kokoro:force-run Forces CI to rerun label May 8, 2024

kokoro-team removed the kokoro:force-run Forces CI to rerun label May 8, 2024

penpornk approved these changes May 8, 2024

View reviewed changes

ShengYang1 force-pushed the yang/llvm-spirv branch from d454eb2 to 984ac8b Compare May 17, 2024 06:53

github-actions bot added the kokoro:force-run Forces CI to rerun label May 17, 2024

kokoro-team removed the kokoro:force-run Forces CI to rerun label May 17, 2024

penpornk approved these changes May 17, 2024

View reviewed changes

dimitar-asenov requested a review from ddunl May 17, 2024 11:26

akuegel requested a review from cheshire May 28, 2024 08:14

copybara-service bot mentioned this pull request Aug 9, 2024

Automated Code Change tensorflow/tensorflow#73449

Draft

copybara-service bot closed this in c4edf34 Aug 9, 2024

copybara-service bot mentioned this pull request Aug 9, 2024

[XLA] Auto-create dump directory for autotuner cache tensorflow/tensorflow#73296

Merged

copybara-service bot mentioned this pull request Aug 9, 2024

[XLA:GPU] Fix order depended tests in dynamic_slice_fusion_test.cc tensorflow/tensorflow#73455

Merged

copybara-service bot mentioned this pull request Aug 9, 2024

Remove unused proto import tensorflow/tensorflow#73456

Merged

copybara-service bot mentioned this pull request Aug 9, 2024

Automated Code Change tensorflow/tensorflow#73440

Merged

copybara-service bot mentioned this pull request Aug 9, 2024

Automated Code Change tensorflow/tensorflow#73453

Merged

copybara-service bot mentioned this pull request Aug 9, 2024

PR #11424: [XLA:GPU] Add SPIRV-LLVM-Translator and translation pass google/tsl#2530

Merged

copybara-service bot mentioned this pull request Aug 9, 2024

PR #11424: [XLA:GPU] Add SPIRV-LLVM-Translator and translation pass tensorflow/tensorflow#73462

Merged

copybara-service bot mentioned this pull request Aug 9, 2024

Remove an unused Windows Dockerfile. tensorflow/tensorflow#73475

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[XLA:GPU] Add SPIRV-LLVM-Translator and translation pass #11424

[XLA:GPU] Add SPIRV-LLVM-Translator and translation pass #11424

ShengYang1 commented Apr 11, 2024

kamaljeeti commented May 7, 2024

penpornk left a comment

penpornk May 7, 2024

ShengYang1 May 8, 2024

penpornk May 8, 2024

ShengYang1 May 8, 2024

penpornk May 7, 2024

ShengYang1 May 8, 2024

penpornk May 7, 2024

ShengYang1 May 8, 2024

penpornk May 7, 2024

ShengYang1 May 8, 2024

penpornk May 8, 2024

penpornk left a comment

NaiyerRizz commented May 14, 2024

dimitar-asenov commented May 16, 2024

ShengYang1 commented May 17, 2024

penpornk left a comment

dimitar-asenov commented May 17, 2024

akuegel commented May 24, 2024

ShengYang1 commented May 28, 2024

akuegel commented May 28, 2024

dimitar-asenov commented Jul 10, 2024

golechwierowicz commented Aug 9, 2024 •

edited

Loading

golechwierowicz commented Aug 9, 2024

Zantares commented Aug 13, 2024

		@@ -1075,5 +1082,92 @@ absl::StatusOr<std::vector<uint8_t>> CompileToHsaco(

		} // namespace amdgpu

		namespace {

[XLA:GPU] Add SPIRV-LLVM-Translator and translation pass #11424

[XLA:GPU] Add SPIRV-LLVM-Translator and translation pass #11424

Conversation

ShengYang1 commented Apr 11, 2024

kamaljeeti commented May 7, 2024

penpornk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

penpornk left a comment

Choose a reason for hiding this comment

NaiyerRizz commented May 14, 2024

dimitar-asenov commented May 16, 2024

ShengYang1 commented May 17, 2024

penpornk left a comment

Choose a reason for hiding this comment

dimitar-asenov commented May 17, 2024

akuegel commented May 24, 2024

ShengYang1 commented May 28, 2024

akuegel commented May 28, 2024

dimitar-asenov commented Jul 10, 2024

golechwierowicz commented Aug 9, 2024 • edited Loading

golechwierowicz commented Aug 9, 2024

Zantares commented Aug 13, 2024

golechwierowicz commented Aug 9, 2024 •

edited

Loading