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

Generalize reuse #368

Merged
merged 6 commits into from
May 3, 2024
Merged

Generalize reuse #368

merged 6 commits into from
May 3, 2024

Conversation

eldritchconundrum
Copy link
Collaborator

No description provided.

@eldritchconundrum eldritchconundrum merged commit 9cf2314 into master May 3, 2024
1 check passed
@eldritchconundrum eldritchconundrum deleted the generalize-reuse branch May 3, 2024 23:52
@therontarigo
Copy link
Contributor

This is great for fully minified shaders, but confusing when --no-renaming is used:

int test()
{
  int one=1, two=2, three=one+two;
  return three;
}

becomes (with --no-renaming --no-inlining)

int test()
{
  int one=1,two=2;
  one+=two;
  return one;
}

should identifiers from the no-renaming-list be excluded from reuse?

@eldritchconundrum
Copy link
Collaborator Author

I think it would be best for renaming to stay independent from rewrites, so that --no-renaming can be used as a debugging step to understand how a minified shader is failing.
Maybe we could make a separate flag to disable variable reuse... Or use --no-renaming-list, I forgot that it also applies to variables.

@therontarigo
Copy link
Contributor

therontarigo commented May 5, 2024

That's a separate issue of --no-renaming-list applying to everything when the help suggests it applies to function names only.

--no-renaming can be used as a debugging step

This is specifically where it's problematic: in large functions, reuse has the same obfuscating effect that renaming does for making debugging more difficult: looking through the tests for examples, "dlt" became "height", "specular" became "lightDir", "dir" became "camTarget", and so forth.

Splitting --no-renaming option into separate options for global renaming vs. local renaming could help solve this, with no-local-renaming implying no local reuse.

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.

None yet

3 participants