Skip to content

Intrinsic-test: Updated the Constraint enum to support discrete values #1892

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

Conversation

madhav-madhusoodanan
Copy link
Contributor

@madhav-madhusoodanan madhav-madhusoodanan commented Aug 2, 2025

Context

This PR is part of the changes that were originally made in the x86 extension PR #1814 for intrinsic-test, and is intended to unblock efforts to support other architectures too.

Further context: #1884 (comment)

cc: @folkertdev @Amanieu

@rustbot
Copy link
Collaborator

rustbot commented Aug 2, 2025

r? @folkertdev

rustbot has assigned @folkertdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@madhav-madhusoodanan
Copy link
Contributor Author

@folkertdev I remember our discussion on whether there might be a better name than "Constraint".

Essentially, this feature is present for arguments that have a restricted set of legal values.

Would a word like "Restriction" or so be more appropriate?

@folkertdev
Copy link
Contributor

I was thinking something more like ConstArgument? That is slightly rust-specific, but really these are just arguments, they just need to be known at compile-time.

Weirdly CI fails in loongarch, that's strange.

@madhav-madhusoodanan
Copy link
Contributor Author

madhav-madhusoodanan commented Aug 2, 2025

I was thinking something more like ConstArgument? That is slightly rust-specific, but really these are just arguments, they just need to be known at compile-time.

In this context though, would it be appropriate?

/// An argument for the intrinsic.
#[derive(Debug, PartialEq, Clone)]
pub struct Argument<T: IntrinsicTypeDefinition> {
    /// The argument's index in the intrinsic function call.
    pub pos: usize,
    /// The argument name.
    pub name: String,
    /// The type of the argument.
    pub ty: T,
    /// Any constraints that are on this argument
    pub constraint: Option<Constraint>,
}

Weirdly CI fails in loongarch, that's strange.

Let me try making another commit, perhaps this might be a one-off occurrence with the CI

@folkertdev
Copy link
Contributor

#1893 fixes the loongarch issue

@folkertdev
Copy link
Contributor

In this context though, would it be appropriate?

Hmm, let's just leave it as Constraint for now then and I'll think about it some more.

@madhav-madhusoodanan madhav-madhusoodanan force-pushed the intrinsic-test-constraint-update branch from 51cf357 to 7dd81ca Compare August 3, 2025 10:03
@folkertdev folkertdev added this pull request to the merge queue Aug 3, 2025
Merged via the queue into rust-lang:master with commit 3ff4f70 Aug 3, 2025
62 checks passed
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.

3 participants