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

webgpu: Remove glsl code #5654

Merged
merged 8 commits into from Sep 24, 2021
Merged

webgpu: Remove glsl code #5654

merged 8 commits into from Sep 24, 2021

Conversation

qjia7
Copy link
Collaborator

@qjia7 qjia7 commented Sep 23, 2021

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.


This change is Reviewable

@qjia7
Copy link
Collaborator Author

qjia7 commented Sep 23, 2021

@axinging @xhcao @haoyunfeix PTAL

@qjia7
Copy link
Collaborator Author

qjia7 commented Sep 23, 2021

@lina128 Do you know why removing WEBGPU_USE_GLSL from benchmarks fails?

}

getUserCode(): string {
getUserCodeWgsl(): string {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't include wgsl in name.

`;
return userCode;
}

getUserCodeWgsl(): string {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we unify the function name to getUserCode?

pipelineLayout: GPUPipelineLayout,
inputsData: shader_preprocessor.InputInfo[], output: TensorInfo,
inputsData: shader_preprocessor_wgsl.InputInfo[], output: TensorInfo,
Copy link
Collaborator

Choose a reason for hiding this comment

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

no wgsl here

Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

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

Hi Jiajia, I haven't dig deep into this, just some ideas here, did you generate new bundle and reference the local bundle in your benchmark tool?

Reviewable status: 0 of 1 approvals obtained (waiting on @pyu10055 and @qjia7)

Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @pyu10055 and @qjia7)

@axinging
Copy link
Contributor

LGTM with nit: please remove glsl_version.ts

@gyagp
Copy link
Collaborator

gyagp commented Sep 24, 2021

LGTM

@qjia7 qjia7 merged commit fc5bfd8 into tensorflow:master Sep 24, 2021
@qjia7 qjia7 deleted the remove_glsl branch September 24, 2021 08:21
pyu10055 pushed a commit that referenced this pull request Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants