Skip to content

[SYCL] Add supplemental tool for SPIR-V to LLVM-IR conversions #5157

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

Merged
merged 11 commits into from
Dec 30, 2021

Conversation

mdtoguchi
Copy link
Contributor

@mdtoguchi mdtoguchi commented Dec 16, 2021

Tool name:
spirv-to-ir-wrapper

Function:
Provides an interface used within the compiler driver toolchain which
converts SPIR-V to LLVM-IR. Using llvm-spirv as the main conversion
tool, the purpose of this is to be able to take binaries and convert
them to LLVM-IR. The conversion only takes place if the input is
SPIR-V, and if the LLVM-IR is provided it is simply copied and
passed through.

This tool is useful for handling generated fat objects/archives that
contain SPIR-V as opposed to typical LLVM-IR. We do not know the
type of files within the device objects, so we use this tool to make
sure that file consumed after unbundling is always LLVM-IR.

Example usage:
spirv-to-ir-wrapper input.spv -o output.bc // creates LLVM-IR output.bc
spirv-to-ir-wrapper input.bc -o output.bc // no conversion, creates output.bc

Tool name:
  spir-to-ir

Function:
  Provides an interface used within the compiler driver toolchain which
  converts SPIR-V to LLVM-IR.  Using llvm-spirv as the main conversion
  tool, the purpose of this is to be able to take binaries and convert
  them to LLVM-IR.  The conversion only takes place if the input is
  SPIR-V, and if the LLVM-IR is provided it is simply copied and
  passed through.

  This tool is useful for handling generated fat objects/archives that
  contain SPIR-V as opposed to typical LLVM-IR.  We do not now the
  type of files within the device objects, so we use this tool to make
  sure that file consumed after unbundling is always LLVM-IR.

Example usage:
  spir-to-ir input.spv -o output.bc  // creates LLVM-IR output.bc
  spir-to-ir input.bc -o output.bc   // no conversion, creates output.bc
@mdtoguchi mdtoguchi requested a review from AGindinson December 16, 2021 01:45
Copy link
Contributor

@AGindinson AGindinson left a comment

Choose a reason for hiding this comment

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

High-level implementation looks perfectly fine, a couple on-the-go thoughts added as comments

@AGindinson AGindinson self-requested a review December 21, 2021 18:19
@mdtoguchi mdtoguchi marked this pull request as ready for review December 21, 2021 18:33
@mdtoguchi mdtoguchi requested review from bader and a team as code owners December 21, 2021 18:33
@bader bader requested a review from AlexeySachkov December 21, 2021 19:54
AGindinson
AGindinson previously approved these changes Dec 28, 2021
Copy link
Contributor

@AGindinson AGindinson left a comment

Choose a reason for hiding this comment

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

The implementation LGTM! On top of the @intel/llvm-reviewers-runtime approval, conceptual approvals from @AlexeySachkov, @AlexeySotkin would be really appreciated.

@AlexeySotkin
Copy link
Contributor

SPIR and SPIR-V are quite different things. I'd recommend to rename spir -> spirv everywhere in the patch.

@bader
Copy link
Contributor

bader commented Dec 29, 2021

@intel/llvm-reviewers-runtime, ping.

Copy link
Contributor

@AlexeySachkov AlexeySachkov left a comment

Choose a reason for hiding this comment

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

Conceptually LGTM, since the option for the translator to silently do nothing in reverse translation if input is already LLVM IR was rejected in the upstream.

@bader bader merged commit abb0f5c into intel:sycl Dec 30, 2021
@bader
Copy link
Contributor

bader commented Dec 30, 2021

@mdtoguchi, build with linking shared LLVM libraries has failed in post-commit. Please, take a look.
https://github.com/intel/llvm/runs/4665661829?check_suite_focus=true

@mdtoguchi
Copy link
Contributor Author

@bader - thanks, fix is here: #5250

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants