Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve performance when compiling small shaders. #6396

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

csyonghe
Copy link
Collaborator

@csyonghe csyonghe commented Feb 19, 2025

Addresses #6358.

This PR address three performance issues when compiling small shaders:

  1. Avoid copying witness table entries that are not getting used during linking.
  2. Avoid copying auto-diff related decorations and derivative functions during linking, if the user modules doesn't use autodiff.
  3. Cache operator overload resolution results on global session, so each new Session doesn't need to repetitively run through overload resolution from scratch.

These three optimizations combined achieves a 75% reduction in the benchmark provided in #6358.

Copilot summary:

This pull request includes several changes aimed at improving the handling of GLSL modules, enhancing the differentiation capabilities, and optimizing the IR linking process. The most important changes are grouped by their themes below:

GLSL Module Handling:

  • Added handling for GLSL modules in the SemanticsVisitor::importModuleIntoScope method to set the glslModuleDecl when the module name is "glsl".
  • Updated OperatorOverloadCacheKey to include an isGLSLMode flag and adjusted its hash code calculation accordingly.
  • Modified ResolveInvoke to set the isGLSLMode flag based on the presence of the GLSL module and cache the resolved operator overloads conditionally. [1] [2]

Differentiation Enhancements:

  • Introduced the IDifferentiable interface with the KnownBuiltin("IDifferentiable") attribute.
  • Changed the NullDifferential struct to remove the export keyword.
  • Updated the method for finding the IDifferentiable interface to use IRKnownBuiltinDecoration instead of IRNameHintDecoration.

IR Linking Optimization:

  • Implemented the ImmutableHashedString class for efficient string handling in the IR linking process.
  • Updated the IRSharedSpecContext::SymbolDictionary to use ImmutableHashedString and added a new dictionary for imported symbols. [1] [2]
  • Enhanced the cloning process for witness tables to handle decorations and entries more efficiently, including a deep clone condition based on specific decorations. [1] [2]

Type Checking Cache:

  • Refactored TypeCheckingCache to inherit from RefObject and updated its usage in Linkage and Session classes. [1] [2] [3]

Debugging and Specialized Targets:

  • Added a GLSL-specific debug break function with the __specialized_for_target(glsl) attribute and corresponding SPIR-V instruction.

These changes collectively improve the handling of GLSL modules, enhance differentiation capabilities, and optimize the IR linking process, making the codebase more efficient and maintainable.

jkwak-work
jkwak-work previously approved these changes Feb 20, 2025
Copy link
Collaborator

@jkwak-work jkwak-work left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Comment on lines +796 to +797
String slice;
HashCode64 hashCode;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably we should make the members private.
Not sure if we want to make them const as well since it is Immutable.

Comment on lines +20312 to 20315
[require(glsl)]
__specialized_for_target(glsl)
[[vk::spirv_instruction(1, "NonSemantic.DebugBreak")]]
void __glslDebugBreak();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should move this to glsl.meta.slang?

IRBuilder builderStorage;

// The "global" specialization environment.
IRSpecEnv globalEnv;
};

void insertGlobalValueSymbol(IRSharedSpecContext* sharedContext, IRInst* gv);

struct WitnessTableCloenInfo : RefObject
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copilot found typo here.
WitnessTableCloenInfo -> WitnessTableCloneInfo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch, how did you setup copilot to do the review?

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ csyonghe
❌ Yong He


Yong He seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
pr: non-breaking PRs without breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants