Skip to content

[Driver][SYCL] Enable adding of default device triple for AOT #4150

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

Closed
wants to merge 1 commit into from

Conversation

mdtoguchi
Copy link
Contributor

When specifying an AOT enabling triple (spir64_gen, spir64_fpga,
spir64_x86_64) without also providing the default triple (spir64),
enable the default triple as well as the AOT enabling triple.

When specifying an AOT enabling triple (spir64_gen, spir64_fpga,
spir64_x86_64) without also providing the default triple (spir64),
enable the default triple as well as the AOT enabling triple.
@mdtoguchi mdtoguchi requested a review from AGindinson as a code owner July 21, 2021 01:35
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.

As far as I understand, the large part of the product code here is about retaining backwards compatibility for AOT users' build scripts, namely the ones relying on shortened versions of -Xsycl-target-* options.

If such a change is required for a particular use scenario, could it be better to hide this behavior under a separate option (-fsycl-force-spirv-image[=false|true] or something)? This would:

  • allow to minimize the code (basically, just adding the generic triple inside the toolchain-building functions on the condition of the new option;
  • push the beneficiaries of such "AOT + SPIR-V behavior" to refactor their build scripts and fix -Xsycl-target-* configuration issues along the way (and it would still be a relatively simple switch). Besides, such users may want to, at some point, also pass -Xsycl-target-* build/link options to the generic images' descriptors. As far as I understand the current approach, this would force them into using the triple explicitly, which would kind of devalue the automation work;
  • additionally, the rest of the AOT users would be immune from unexpected changes to the driver behavior (I can't come up with a quick example, but some backwards compatibility issues could still fire for applications/libraries after the introduction of a second device binary). A 15-40% increase in the executable's size without a possibility to revert to old behavior could also cause headaches for those shipping large heterogeneous libraries.

@mdtoguchi mdtoguchi marked this pull request as draft July 22, 2021 16:00
@mdtoguchi
Copy link
Contributor Author

closing - replaced with #4175

@mdtoguchi mdtoguchi closed this Jul 24, 2021
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.

2 participants