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

Support for a scalable simd representation #118917

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/rustc_abi/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ where
hide_niches(b);
}
Abi::Vector { element, count: _ } => hide_niches(element),
Abi::ScalableVector { element, .. } => hide_niches(element),
Abi::Aggregate { sized: _ } => {}
}
st.largest_niche = None;
Expand Down
26 changes: 24 additions & 2 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,11 @@ bitflags! {
// If true, the type's layout can be randomized using
// the seed stored in `ReprOptions.field_shuffle_seed`
const RANDOMIZE_LAYOUT = 1 << 4;
const IS_SCALABLE = 1 << 5;
// Any of these flags being set prevent field reordering optimisation.
const IS_UNOPTIMISABLE = ReprFlags::IS_C.bits()
| ReprFlags::IS_SIMD.bits()
| ReprFlags::IS_SCALABLE.bits()
| ReprFlags::IS_LINEAR.bits();
}
}
Expand Down Expand Up @@ -90,6 +92,7 @@ pub struct ReprOptions {
pub align: Option<Align>,
pub pack: Option<Align>,
pub flags: ReprFlags,
pub scalable: Option<u32>,
/// The seed to be used for randomizing a type's layout
///
/// Note: This could technically be a `u128` which would
Expand All @@ -106,6 +109,11 @@ impl ReprOptions {
self.flags.contains(ReprFlags::IS_SIMD)
}

#[inline]
pub fn scalable(&self) -> bool {
self.flags.contains(ReprFlags::IS_SCALABLE)
}

#[inline]
pub fn c(&self) -> bool {
self.flags.contains(ReprFlags::IS_C)
Expand Down Expand Up @@ -1306,6 +1314,10 @@ pub enum Abi {
Uninhabited,
Scalar(Scalar),
ScalarPair(Scalar, Scalar),
ScalableVector {
element: Scalar,
elt: u64,

Choose a reason for hiding this comment

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

nit: elt -> elt_cnt?

},
Vector {
element: Scalar,
count: u64,
Expand All @@ -1323,6 +1335,7 @@ impl Abi {
match *self {
Abi::Uninhabited | Abi::Scalar(_) | Abi::ScalarPair(..) | Abi::Vector { .. } => false,
Abi::Aggregate { sized } => !sized,
Abi::ScalableVector { .. } => true,
}
}

Expand Down Expand Up @@ -1369,7 +1382,7 @@ impl Abi {
Abi::Vector { element, count } => {
cx.data_layout().vector_align(element.size(cx) * count)
}
Abi::Uninhabited | Abi::Aggregate { .. } => return None,
Abi::Uninhabited | Abi::Aggregate { .. } | Abi::ScalableVector { .. } => return None,
})
}

Expand All @@ -1390,7 +1403,7 @@ impl Abi {
// to make the size a multiple of align (e.g. for vectors of size 3).
(element.size(cx) * count).align_to(self.inherent_align(cx)?.abi)
}
Abi::Uninhabited | Abi::Aggregate { .. } => return None,
Abi::Uninhabited | Abi::Aggregate { .. } | Abi::ScalableVector { .. } => return None,
})
}

Expand All @@ -1400,6 +1413,9 @@ impl Abi {
Abi::Scalar(s) => Abi::Scalar(s.to_union()),
Abi::ScalarPair(s1, s2) => Abi::ScalarPair(s1.to_union(), s2.to_union()),
Abi::Vector { element, count } => Abi::Vector { element: element.to_union(), count },
Abi::ScalableVector { element, elt } => {
Abi::ScalableVector { element: element.to_union(), elt }
}
Abi::Uninhabited | Abi::Aggregate { .. } => Abi::Aggregate { sized: true },
}
}
Expand Down Expand Up @@ -1686,6 +1702,11 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutS<FieldIdx, VariantIdx> {
self.is_sized() && self.size.bytes() == 0 && self.align.abi.bytes() == 1
}

/// Returns true if the size of the type is only known at runtime.
pub fn is_runtime_sized(&self) -> bool {
matches!(self.abi, Abi::ScalableVector { .. })
Comment on lines +1705 to +1707
Copy link
Member

Choose a reason for hiding this comment

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

This coexisting with is_unsized feels potentially misleading, or at the very least ambiguous.

Also, the possibility of knowing the size at all feels like an edge case out of all possible "value-only"/"memory-unrepresentable" types discussed in e.g. these older RFC comments:

As far as Abi knowing about these types, that's an inevitability (since they do participate in ABI), and sadly I don't know of a good way to encode open-ended target-specific "handle" types (other than e.g. an interned [Either<Symbol, u64>] style sequence, comparable to specifying register names using strings in asm! etc.).

}

/// Returns `true` if the type is a ZST and not unsized.
///
/// Note that this does *not* imply that the type is irrelevant for layout! It can still have
Expand All @@ -1695,6 +1716,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutS<FieldIdx, VariantIdx> {
Abi::Scalar(_) | Abi::ScalarPair(..) | Abi::Vector { .. } => false,
Abi::Uninhabited => self.size.bytes() == 0,
Abi::Aggregate { sized } => sized && self.size.bytes() == 0,
Abi::ScalableVector { .. } => false,
}
}

Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,15 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
"SIMD types are experimental and possibly buggy"
);
}

if item.has_name(sym::scalable) {
gate!(
&self,
repr_scalable,
attr.span,
"Scalable SIMD types are experimental and possibly buggy"
Copy link

@michaelmaitland michaelmaitland Jun 27, 2024

Choose a reason for hiding this comment

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

Why do we call these SIMD? Is it because they are part of the simd crate? If so, that's unfortunate since SVE and RVV is not SIMD. The these extensions allow the vector length to vary at runtime whereas with SIMD the vector length is fixed at compile time. The best term is scalable vector types.

I'm not suggesting or requesting a change here, just making an observation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the term scalable vector types can also cause confusion. I have spoken with a few people that when you say scalable vectors, they think it's something to with vectors in the sense of a Vec not these types, although there would be context here.

Saying they allow the vector length to change at runtime is not correct here, that would be considered undefined behavior with SVE in LLVM and Rust. With SVE the vector length isn't changed, rather lanes are masked off. Changing vector length at runtime for SVE could lead to an incorrect stack if any SVE register was spilled to the stack. During execution SVE should be a fixed vector size with lanes ignored (for predicated instructions) based on the governing predicate.

);
}
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_attr/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ attr_rustc_allowed_unstable_pairing =
attr_rustc_promotable_pairing =
`rustc_promotable` attribute must be paired with either a `rustc_const_unstable` or a `rustc_const_stable` attribute

attr_scalable_missing_n =
invalid `scalable(num)` attribute: `scalable` needs an argument
.suggestion = supply an argument here
attr_soft_no_args =
`soft` should not have any arguments

Expand Down
27 changes: 26 additions & 1 deletion compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,7 @@ pub enum ReprAttr {
ReprSimd,
ReprTransparent,
ReprAlign(Align),
ReprScalable(u32),
}

#[derive(Eq, PartialEq, Debug, Copy, Clone)]
Expand Down Expand Up @@ -973,6 +974,13 @@ pub fn parse_repr_attr(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
recognised = true;
None
}
sym::scalable => {
sess.dcx().emit_err(session_diagnostics::ScalableAttrMissingN {
span: item.span(),
});
recognised = true;
None
}
name => int_type_of_word(name).map(ReprInt),
};

Expand Down Expand Up @@ -1001,6 +1009,12 @@ pub fn parse_repr_attr(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
literal_error = Some(message)
}
};
} else if name == sym::scalable {
recognised = true;
match parse_scalable(&value.kind) {
Ok(literal) => acc.push(ReprScalable(literal)),
Err(message) => literal_error = Some(message),
};
} else if matches!(name, sym::Rust | sym::C | sym::simd | sym::transparent)
|| int_type_of_word(name).is_some()
{
Expand All @@ -1020,7 +1034,10 @@ pub fn parse_repr_attr(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
} else if let Some(meta_item) = item.meta_item() {
match &meta_item.kind {
MetaItemKind::NameValue(value) => {
if meta_item.has_name(sym::align) || meta_item.has_name(sym::packed) {
if meta_item.has_name(sym::align)
|| meta_item.has_name(sym::packed)
|| meta_item.has_name(sym::scalable)
{
let name = meta_item.name_or_empty().to_ident_string();
recognised = true;
sess.dcx().emit_err(session_diagnostics::IncorrectReprFormatGeneric {
Expand Down Expand Up @@ -1239,3 +1256,11 @@ pub fn parse_confusables(attr: &Attribute) -> Option<Vec<Symbol>> {

return Some(candidates);
}

pub fn parse_scalable(node: &ast::LitKind) -> Result<u32, &'static str> {
if let ast::LitKind::Int(literal, ast::LitIntType::Unsuffixed) = node {
(literal.get()).try_into().map_err(|_| "integer too large")
} else {
Err("not an unsuffixed integer")
}
}
8 changes: 8 additions & 0 deletions compiler/rustc_attr/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,3 +401,11 @@ pub(crate) struct UnknownVersionLiteral {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(attr_scalable_missing_n, code = E0799)]
pub(crate) struct ScalableAttrMissingN {
#[primary_span]
#[suggestion(applicability = "has-placeholders", code = "scalable(...)")]
pub span: Span,
}
28 changes: 20 additions & 8 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,15 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
place_ty = self.sanitize_projection(place_ty, elem, place, location, context);
}

if let PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) = context {
// The Copy trait isn't implemented for scalable SIMD types.
// These types live somewhere between `Sized` and `Unsize`.
// The bounds on `Copy` disallow the trait from being
// implemented for them. As a result of this no bounds from
// `Copy` apply for the type, therefore, skipping this check
// should be perfectly legal.
if let PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) = context
&& !place_ty.ty.is_scalable_simd()
{
let tcx = self.tcx();
let trait_ref = ty::TraitRef::new(
tcx,
Expand Down Expand Up @@ -1267,7 +1275,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}

self.check_rvalue(body, rv, location);
if !self.unsized_feature_enabled() {
if !(self.unsized_feature_enabled() || place_ty.is_scalable_simd()) {
let trait_ref = ty::TraitRef::new(
tcx,
tcx.require_lang_item(LangItem::Sized, Some(self.last_span)),
Expand Down Expand Up @@ -1788,7 +1796,9 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
if !self.unsized_feature_enabled() {
let span = local_decl.source_info.span;
let ty = local_decl.ty;
self.ensure_place_sized(ty, span);
if !ty.is_scalable_simd() {
self.ensure_place_sized(ty, span);
}
}
}

Expand All @@ -1804,11 +1814,13 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
// expressions evaluate through `as_temp` or `into` a return
// slot or local, so to find all unsized rvalues it is enough
// to check all temps, return slots and locals.
if self.reported_errors.replace((ty, span)).is_none() {
// While this is located in `nll::typeck` this error is not
// an NLL error, it's a required check to prevent creation
// of unsized rvalues in a call expression.
self.tcx().dcx().emit_err(MoveUnsized { ty, span });
if !ty.is_scalable_simd() {
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed, it's already handled in the callers of ensure_place_sized.

if self.reported_errors.replace((ty, span)).is_none() {
// While this is located in `nll::typeck` this error is not
// an NLL error, it's a required check to prevent creation
// of unsized rvalues in a call expression.
self.tcx().dcx().emit_err(MoveUnsized { ty, span });
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_gcc/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ impl GccType for Reg {
_ => bug!("unsupported float: {:?}", self),
},
RegKind::Vector => unimplemented!(), //cx.type_vector(cx.type_i8(), self.size.bytes()),
RegKind::ScalableVector => unimplemented!(),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/intrinsic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
let layout = self.layout_of(tp_ty).layout;
let _use_integer_compare = match layout.abi() {
Scalar(_) | ScalarPair(_, _) => true,
Uninhabited | Vector { .. } => false,
Uninhabited | Vector { .. } | ScalableVector { .. } => false,
Aggregate { .. } => {
// For rusty ABIs, small aggregates are actually passed
// as `RegKind::Integer` (see `FnAbi::adjust_for_abi`),
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_codegen_gcc/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ fn uncached_gcc_type<'gcc, 'tcx>(
false,
);
}
Abi::ScalableVector { .. } => todo!(),
Abi::Uninhabited | Abi::Aggregate { .. } => {}
}

Expand Down Expand Up @@ -175,15 +176,19 @@ pub trait LayoutGccExt<'tcx> {
impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> {
fn is_gcc_immediate(&self) -> bool {
match self.abi {
Abi::Scalar(_) | Abi::Vector { .. } => true,
Abi::Scalar(_) | Abi::Vector { .. } | Abi::ScalableVector { .. } => true,
Abi::ScalarPair(..) | Abi::Uninhabited | Abi::Aggregate { .. } => false,
}
}

fn is_gcc_scalar_pair(&self) -> bool {
match self.abi {
Abi::ScalarPair(..) => true,
Abi::Uninhabited | Abi::Scalar(_) | Abi::Vector { .. } | Abi::Aggregate { .. } => false,
Abi::Uninhabited
| Abi::Scalar(_)
| Abi::Vector { .. }
| Abi::ScalableVector { .. }
| Abi::Aggregate { .. } => false,
}
}

Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_codegen_llvm/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ impl LlvmType for Reg {
_ => bug!("unsupported float: {:?}", self),
},
RegKind::Vector => cx.type_vector(cx.type_i8(), self.size.bytes()),
// Generate a LLVM type such as <vscale x 16 x i8>, like above for a non scalable
// vector. The use of 16 here is chosen as that will generate a valid type with both
// Arm SVE and RISC-V RVV. In the future with other architectures this might not be
// valid and might have to be configured by the target.
RegKind::ScalableVector => cx.type_scalable_vector(cx.type_i8(), 16),
Copy link
Member

Choose a reason for hiding this comment

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

Could use a comment.

Choose a reason for hiding this comment

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

Why do we use i8 and 16 instead of basing it off of the original type information?

}
}
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,10 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
panic!("unsized locals must not be `extern` types");
}
}
assert_eq!(place.val.llextra.is_some(), place.layout.is_unsized());

if !place.layout.is_runtime_sized() {
assert_eq!(place.val.llextra.is_some(), place.layout.is_unsized());
}
Comment on lines +541 to +543
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if !place.layout.is_runtime_sized() {
assert_eq!(place.val.llextra.is_some(), place.layout.is_unsized());
}
assert_eq!(place.val.llextra.is_some(), place.layout.is_unsized() && !place.layout.is_runtime_sized());


if place.layout.is_zst() {
return OperandRef::zero_sized(place.layout);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,7 @@ fn build_struct_type_di_node<'ll, 'tcx>(
Cow::Borrowed(f.name.as_str())
};
let field_layout = struct_type_and_layout.field(cx, i);

Choose a reason for hiding this comment

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

nit: unrelated newline?

build_field_di_node(
cx,
owner,
Expand Down
Loading
Loading