Skip to content

Conversation

@sarnex
Copy link
Contributor

@sarnex sarnex commented Nov 12, 2024

Use the SPV_INTEL_maximum_registers extension to implement per-kernel GRF selection. Previously we were doing it was a hack adding an annotation on the entry point which only worked for the scalar IGC backend.

This extension has long been supported in scalar IGC, and it never worked previously for ESIMD using the old path and support was recently added for ESIMD using this extension.

There should be no ABI break, previously-generated object files and executable generated with an old compiler will still have the same behavior as before this change.

AttrKindStr == SYCL_GRF_SIZE_ATTR) &&
!llvm::esimd::isESIMD(F)) {
if (AttrKindStr == SYCL_REGISTER_ALLOC_MODE_ATTR ||
AttrKindStr == SYCL_GRF_SIZE_ATTR) {
Copy link
Contributor Author

@sarnex sarnex Nov 13, 2024

Choose a reason for hiding this comment

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

Previously, the translator was looking for RegisterAllocMode MD to be 0,1 or 2. With the new extension enabled, the translator looks for the actual GRF register size as the operand of RegisterAllocMode, which the sycl-grf-size attr already has, so now we need to fixup the old sycl-register-alloc-mode attr value to have the register size instead of the enum value.

Also, the SPV_INTEL_maximum_registers spec does not have a restriction on the value of the operand, so remove that restriction (we error for anything because 0, 128. or 256 in SYCL headers anyway)

We also remove the ESIMD check as the new extension is supported in the vector backend (though that driver is not in our CI yet, but as I said it never worked previously anyway).

@sarnex sarnex marked this pull request as ready for review November 13, 2024 15:46
@sarnex sarnex requested review from a team as code owners November 13, 2024 15:46
@sarnex sarnex requested review from MrSidims and vmaksimo November 13, 2024 15:46
@sarnex sarnex changed the title [SYCL][Driver] Enable SPV_INTEL_maximum_registers by default [SYCL] Enable SPV_INTEL_maximum_registers by default Nov 13, 2024
@sarnex
Copy link
Contributor Author

sarnex commented Nov 13, 2024

Sorry, actually the scalar backend support isn't working, so we can't merge this yet, closing for now.

@sarnex sarnex closed this Nov 13, 2024
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.

1 participant