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

glsl-out: Perform casts in int only math functions #1978

Merged
merged 3 commits into from
Jun 14, 2022

Conversation

JCapucho
Copy link
Collaborator

@JCapucho JCapucho commented Jun 7, 2022

Some functions like abs only accept signed integers while naga's IR
accepts both signed and unsigned integers, this wasn't accounted for in
the glsl backend causing it to produce code that wouldn't type check.

This commit addresses it by performing casts from uint to int of the
function argument and then back from int to uint for the function
return.

closes #1968

Edit: found some problems that need to be fixed first

Edit2: Good to go now

@JCapucho JCapucho marked this pull request as draft June 7, 2022 16:23
@JCapucho JCapucho force-pushed the glsl-out-int-only-functions branch from 2fdc50b to 850cb13 Compare June 7, 2022 20:23
@JCapucho JCapucho marked this pull request as ready for review June 7, 2022 20:24
@teoxoy
Copy link
Member

teoxoy commented Jun 11, 2022

Can we add a test for this?

@JCapucho
Copy link
Collaborator Author

JCapucho commented Jun 12, 2022

Can we add a test for this?

@teoxoy done

src/back/glsl/mod.rs Outdated Show resolved Hide resolved
src/back/glsl/mod.rs Outdated Show resolved Hide resolved
src/back/glsl/mod.rs Outdated Show resolved Hide resolved
src/back/glsl/mod.rs Outdated Show resolved Hide resolved
@teoxoy
Copy link
Member

teoxoy commented Jun 13, 2022

Behavior looks good; just a few things that I think would make the code easier to read.

Copy link
Member

@jimblandy jimblandy left a comment

Choose a reason for hiding this comment

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

Just marking this as "changes requested" per @teoxoy's comments, to make it easier to track status.

Some functions like abs only accept signed integers while naga's IR
accepts both signed and unsigned integers, this wasn't accounted for in
the glsl backend causing it to produce code that wouldn't type check.

This commit addresses it by performing casts from uint to int of the
function argument and then back from int to uint for the function
return.
@JCapucho JCapucho force-pushed the glsl-out-int-only-functions branch from c470489 to 2cd5157 Compare June 14, 2022 21:30
@JCapucho
Copy link
Collaborator Author

Thanks @teoxoy, could you please re-review?

@teoxoy
Copy link
Member

teoxoy commented Jun 14, 2022

Thanks, @jimblandy; not sure what happened there; I didn't mean to review 4 times 😅

@jimblandy jimblandy merged commit 98bc8fe into gfx-rs:master Jun 14, 2022
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.

[glsl-out] Invalid GLSL emission when using firstLeadingBit and others
3 participants