Skip to content
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

[Intel OpenCL CPU] Unit_deviceFunctions_CompileTest_j0f_float fails with global is external, but doesn't have external or weak linkage! #455

Closed
pjaaskel opened this issue May 23, 2023 · 8 comments · Fixed by #466

Comments

@pjaaskel
Copy link
Collaborator

pjaaskel commented May 23, 2023

Unit_deviceFunctions_CompileTest_j0f_float fails with `Global is external, but doesn't have external or weak linkage!

This might be yet another SPIRV-Translator builtin that is generated wrongly without the external attribute.

Also these tests seem to fail similarly:

	129 - Unit_deviceFunctions_CompileTest_j0f_float (Subprocess aborted)
	130 - Unit_deviceFunctions_CompileTest_j1f_float (Subprocess aborted)
	131 - Unit_deviceFunctions_CompileTest_jnf_float (Subprocess aborted)
	178 - Unit_deviceFunctions_CompileTest_y0f_float (Subprocess aborted)
	179 - Unit_deviceFunctions_CompileTest_y1f_float (Subprocess aborted)
	180 - Unit_deviceFunctions_CompileTest_ynf_float (Subprocess aborted)
@pjaaskel pjaaskel changed the title [llvm 16][Intel OpenCL CPU] Unit_deviceFunctions_CompileTest_j0f_float fails with global is external, but doesn't have external or weak linkage! [Intel OpenCL CPU] Unit_deviceFunctions_CompileTest_j0f_float fails with global is external, but doesn't have external or weak linkage! May 23, 2023
@pvelesko
Copy link
Collaborator

These are passing for me. Are you using 2023.1 oneapi?

@pjaaskel
Copy link
Collaborator Author

I'm using the latest drivers from the compute-runtime repo.

@pvelesko
Copy link
Collaborator

Then I'm not sure. I remember bringing these issues to you and then 2023.1 + SPIR-V Patches seems to have resolved these issues

@pjaaskel pjaaskel self-assigned this May 26, 2023
@pjaaskel
Copy link
Collaborator Author

Seems to be due to illegal debug info generated to SPIR-V:

spirv-val j0f-hip-spirv64-generic.out
error: line 543: OpenCL.DebugInfo.100 DebugSource: expected operand Text must be a result id of OpString
  %578 = OpExtInst %void %2 DebugSource %576 %577
...
        %577 = OpExtInst %void %2 DebugInfoNone
        %578 = OpExtInst %void %2 DebugSource %576 %577
...

That is, it presents the missing optional parameter with DebugInfoNone although it should be omitted completely (since it's an optional one) or perhaps described as OpString None or such.

@pjaaskel
Copy link
Collaborator Author

...or not. Seems valid usage:

DebugInfoNone

Other instructions can refer to this one in case the debugging information is unknown, not available, or not applicable

https://registry.khronos.org/SPIR-V/specs/unified1/OpenCL.DebugInfo.100.html#DebugInfoNone

@pjaaskel
Copy link
Collaborator Author

pjaaskel commented May 26, 2023

Yep, that was OK. After fixing two cases of optional args marked with DebugInfoNone, spirv-val passes.

Sent PR KhronosGroup/SPIRV-Tools#5245 to fix these.

@pjaaskel
Copy link
Collaborator Author

This is likely the fshl intrinsic issue that is fixed in our SPIRV-Translator patch and my Intel OpenCL just (of course) uses the unpatched SPIRV-Translator. I'll upstream and see if it gets propagated.

pjaaskel added a commit to pjaaskel/chip-spv that referenced this issue May 29, 2023
Might need upstreaming fixes to SPIR-V translator so the Intel CPU driver
receives them. To investigate.

See CHIP-SPV#455
@pjaaskel
Copy link
Collaborator Author

This might actually be our SPIR-V filter bug introduced in (037ef3e), which strips the extern linkage from the intrinsic placeholder functions, causing the illegal module which some implementations fixup on the fly or just ignore. @linehill 's looking at it.

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

Successfully merging a pull request may close this issue.

2 participants