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

Improve performance when compiling small shaders. #6396

Merged
merged 18 commits into from
Feb 23, 2025

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.

@csyonghe csyonghe added the pr: non-breaking PRs without breaking changes label Feb 19, 2025
@csyonghe csyonghe requested a review from a team as a code owner February 19, 2025 01:49
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

CLAassistant commented Feb 21, 2025

CLA assistant check
All committers have signed the CLA.

@csyonghe csyonghe merged commit 51ad07d into shader-slang:master Feb 23, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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