-
Notifications
You must be signed in to change notification settings - Fork 78
Change SPV_KHR_bfloat16 workaround patch to work for different drivers #5585
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
base: main
Are you sure you want to change the base?
Conversation
|
Do you know why there are no artifacts in the second attempts? Only the first attempt run have the artifacts of the triton pacakge, but what within is |
|
Please don’t try yet, noticed from CI failures that still need more changes. |
1c260fb to
43b020f
Compare
43b020f to
7d3d898
Compare
|
@Stonepia It should be good now, please give it another try. |
third_party/intel/cmake/3122.patch
Outdated
| - return mapType(T, BM->addFloatType(16, FPEncodingBFloat16KHR)); | ||
| + // Workaround for LTS2 driver. | ||
| + const char *driverVersion = std::getenv("INTEL_XPU_BACKEND_DRIVER_VERSION"); | ||
| + if (driverVersion && std::string(driverVersion) != "1.6.33578+38") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the LTS2 driver is updated, this code will break. Is it enough to use some substring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know what's the driver version pattern? Are you suggesting to omit +38?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately no, my guess is that only this part will not change: 1.6.*
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw 1.6.35096+9 on https://github.com/intel/intel-xpu-backend-for-triton/actions/runs/19873342424/job/56954531877 which uses Rolling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If LTS2 driver is updated, it may include SPV_KHR_bfloat16 extension, and no longer need to have this workaround anyways?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If LTS2 driver is updated, it may include SPV_KHR_bfloat16 extension, and no longer need to have this workaround anyways?
It depends on the update strategy. If it's just fixes, then this shouldn't be added, since it seems more like a feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what about std::string(driverVersion) < "1.6.35096+9"?
43d94d6 to
c63305b
Compare
c63305b to
a5d6a6f
Compare
Check the driver version at runtime instead of build time.
Fixes #5574