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

Name fixup is undone by replace_illegal_names in the case of entry points #1571

Closed
VZout opened this issue Dec 18, 2020 · 0 comments · Fixed by #1576
Closed

Name fixup is undone by replace_illegal_names in the case of entry points #1571

VZout opened this issue Dec 18, 2020 · 0 comments · Fixed by #1576
Labels
bug Feature which should work in SPIRV-Cross does not for some reason. in progress Issue is being actively worked on

Comments

@VZout
Copy link

VZout commented Dec 18, 2020

Currently fixup_reserved_names fixes names to not include invalid characters. However this fixup is undone later by replace_illegal_names.

See the offending line: https://github.com/KhronosGroup/SPIRV-Cross/blob/master/spirv_msl.cpp#L12189

This has the side effect of also breaking structs with invalid names if a entry points returns that struct.

Possible solution is calling fixup_reserved_names (https://github.com/KhronosGroup/SPIRV-Cross/blob/master/spirv_msl.cpp#L1183) after replace_illegal_names (https://github.com/KhronosGroup/SPIRV-Cross/blob/master/spirv_msl.cpp#L1231)

Another possible solution is writing

if (illegal_func_names.find(ep_name) != end(illegal_func_names))
{
	ep_name += "0";
	ir.meta[entry.first].decoration.alias += "0";
}

instead of

string &ep_name = entry.second.name;
if (illegal_func_names.find(ep_name) != end(illegal_func_names))
{
	ep_name += "0";
	ir.meta[entry.first].decoration.alias += "0";
}

// Always write this because entry point might have been renamed earlier.
ir.meta[entry.first].decoration.alias = ep_name;

What do y'all think? This is currently a major issue for https://github.com/EmbarkStudios/rust-gpu

@HansKristian-Work HansKristian-Work added out-of-office-maintainer Issue will be looked at after vacation needs triage Needs to be reproduced before it can become a different issue type. labels Dec 22, 2020
@HansKristian-Work HansKristian-Work added bug Feature which should work in SPIRV-Cross does not for some reason. in progress Issue is being actively worked on and removed out-of-office-maintainer Issue will be looked at after vacation needs triage Needs to be reproduced before it can become a different issue type. labels Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Feature which should work in SPIRV-Cross does not for some reason. in progress Issue is being actively worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants