Skip to content

Fix stride in test res-in-struct-mix#734

Merged
joaosaffran merged 5 commits intollvm:mainfrom
joaosaffran:bugfix/amd-res-in-struct-mix
Feb 13, 2026
Merged

Fix stride in test res-in-struct-mix#734
joaosaffran merged 5 commits intollvm:mainfrom
joaosaffran:bugfix/amd-res-in-struct-mix

Conversation

@joaosaffran
Copy link
Contributor

@joaosaffran joaosaffran commented Feb 12, 2026

The stride was wrong for res-in-struct-mix.test, this patches fixes it making it pass in AMD GPUs
Also Fix Out Buffer type from RWBuffer to RWStructuredBuffer.

fixes: #655
fixes #643

Copy link
Contributor

@bob80905 bob80905 left a comment

Choose a reason for hiding this comment

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

LGTM

Binding: 6
- Name: BufOut
Kind: RWBuffer
Kind: RWStructuredBuffer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this change? This isn't changing the stride.

Copy link
Member

Choose a reason for hiding this comment

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

Because its another bug in the test see line 20: RWStructuredBuffer<int4> Out : register(u0);

Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be mentioned in the description, otherwise it looks like an accidental change.

Copy link
Contributor

@bob80905 bob80905 left a comment

Choose a reason for hiding this comment

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

One nit

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, this comment doesn't seem to be accurate. There are no resource arrays, only resources, and these are classes, not structs.

@farzonl farzonl added the test-all When applied to a PR this will opt-in to additional pre-merge test configurations.. label Feb 12, 2026
@farzonl
Copy link
Member

farzonl commented Feb 12, 2026

@joaosaffran you will need to push another commit or rebase. the test-all tag was placed after the CI started so the AMD gpu machine didn't run your pr.

Copy link
Member

@farzonl farzonl left a comment

Choose a reason for hiding this comment

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

Seeing both amd and intel passing now. LGTM!

@farzonl
Copy link
Member

farzonl commented Feb 12, 2026

add a fix line for #643 aswell

@joaosaffran joaosaffran merged commit dc7681b into llvm:main Feb 13, 2026
20 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test-all When applied to a PR this will opt-in to additional pre-merge test configurations..

Projects

None yet

6 participants