Skip to content

tests: codegen-llvm: Update bpf-alu32 with the new LLVM attributes#157249

Open
vadorovsky wants to merge 3 commits into
rust-lang:mainfrom
vadorovsky:vad/llvm-codegen-bpf-alu32
Open

tests: codegen-llvm: Update bpf-alu32 with the new LLVM attributes#157249
vadorovsky wants to merge 3 commits into
rust-lang:mainfrom
vadorovsky:vad/llvm-codegen-bpf-alu32

Conversation

@vadorovsky
Copy link
Copy Markdown
Contributor

@vadorovsky vadorovsky commented Jun 1, 2026 •
edited
Loading

The LLVM backend now emits noundef zeroext on i8 return values and noundef on i8 parameters. Update the FileCheck pattern to match.

r? @nagisa

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jun 1, 2026

Failed to set assignee to alessandrod: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 1, 2026
nikic
nikic reviewed Jun 1, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@nikic nikic left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

Can you please convert this to a minicore test as well? That way it'll never get outdated again in the future.

View changes since this review

@vadorovsky
Copy link
Copy Markdown
Contributor Author

vadorovsky commented Jun 1, 2026

Can you please convert this to a minicore test as well? That way it'll never get outdated again in the future.

Done.

Another thing - should I use {{.*}} instead of hard coding the attributes? Just thought of it now, while looking at the other tests.

@nikic
Copy link
Copy Markdown
Contributor

nikic commented Jun 1, 2026

It looks like the test was added in 9cf2170 and the purpose seems to basically just be "target features on bpf work". TBH I'm not sure this should be a codegen test at all (at least then it should probably actually check for the "target-features" attribute?)

But yes, as this is not actually intending to test any ABI behavior on BPF, using {{.*}} would be fine here.

@nagisa
Copy link
Copy Markdown
Member

nagisa commented Jun 1, 2026

Agreed, I'm pretty sure we can {{.*}} most of these attributes.

@rust-bors

This comment has been minimized.

Sign in to view
@vadorovsky
Copy link
Copy Markdown
Contributor Author

vadorovsky commented Jun 2, 2026

It looks like the test was added in 9cf2170 and the purpose seems to basically just be "target features on bpf work". TBH I'm not sure this should be a codegen test at all (at least then it should probably actually check for the "target-features" attribute?)

I would keep the codegen test, but I agree with adding a check for the "target-features" attribute, so it actually tests whether the feature ends up in the IR.

That said, I also think it would be nice to have a separate UI test for the #[target_feature(enable = "alu32")] attribute. I can add one (as a separate commit). How does it sound to you?

@nikic
Copy link
Copy Markdown
Contributor

nikic commented Jun 2, 2026

That said, I also think it would be nice to have a separate UI test for the #[target_feature(enable = "alu32")] attribute. I can add one (as a separate commit). How does it sound to you?

Not strictly opposed, but I'm not sure what additional value an UI test would add over the codegen test?

vadorovsky added 3 commits June 2, 2026 14:51
@vadorovsky
tests: codegen-llvm: Allow additional LLVM attributes in bpf-alu32
c3cfc85 
The LLVM backend now emits `noundef zeroext` on `i8` return values and
`noundef` on `i8` parameters. Update the FileCheck pattern to match
those, and any possible future attributes.
@vadorovsky
tests: codegen-llvm: Use minicore in bpf-alu32
c757378 
Instead of keeping it as `only-bpf`, use minicore. This way we make sure
it will never get outdated again.
@vadorovsky
tests: codegen-llvm: Check whether feature is emitted in bpf-alu32 test
06edf3d 
Check whether the `"target-feature"` attribure was actually emitted in
LLVM IR.
@vadorovsky vadorovsky force-pushed the vad/llvm-codegen-bpf-alu32 branch from c249a60 to 06edf3d Compare June 2, 2026 12:53
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jun 2, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@vadorovsky
Copy link
Copy Markdown
Contributor Author

vadorovsky commented Jun 2, 2026

Not strictly opposed, but I'm not sure what additional value an UI test would add over the codegen test?

Probably none. I adjusted the codegen test instead, PTAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikic nikic nikic left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vadorovsky @rustbot @nikic @nagisa