-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add rustc_intrinsic_const_vector_arg attribute to allow vectors to be passed as constants #118980
Changes from all commits
06b2dc0
ca26d33
859db1e
489c449
860f66e
e0f2933
0c13d85
6560527
4a49ae6
25f9653
a98491c
0309238
6961ec6
9bffa2f
4b09873
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ use super::{CachedLlbb, FunctionCx, LocalRef}; | |
|
||
use crate::base::{self, is_call_from_compiler_builtins_to_upstream_monomorphization}; | ||
use crate::common::{self, IntPredicate}; | ||
use crate::errors::CompilerBuiltinsCannotCall; | ||
use crate::errors; | ||
use crate::meth; | ||
use crate::traits::*; | ||
use crate::MemFlags; | ||
|
@@ -169,7 +169,7 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { | |
if destination.is_some() { | ||
let caller = with_no_trimmed_paths!(tcx.def_path_str(fx.instance.def_id())); | ||
let callee = with_no_trimmed_paths!(tcx.def_path_str(instance.def_id())); | ||
tcx.dcx().emit_err(CompilerBuiltinsCannotCall { caller, callee }); | ||
tcx.dcx().emit_err(errors::CompilerBuiltinsCannotCall { caller, callee }); | ||
} else { | ||
info!( | ||
"compiler_builtins call to diverging function {:?} replaced with abort", | ||
|
@@ -925,7 +925,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |
// checked by the type-checker. | ||
if i == 2 && intrinsic.name == sym::simd_shuffle { | ||
if let mir::Operand::Constant(constant) = &arg.node { | ||
let (llval, ty) = self.simd_shuffle_indices(bx, constant); | ||
let (llval, ty) = self.early_evaluate_const_vector(bx, constant); | ||
|
||
return OperandRef { | ||
val: Immediate(llval), | ||
layout: bx.layout_of(ty), | ||
|
@@ -1003,9 +1004,32 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |
(args, None) | ||
}; | ||
|
||
let const_vec_arg_indexes = if let Some(def) = def { | ||
let val = bx.tcx().codegen_fn_attrs(def.def_id()); | ||
let const_vector_indices = &val.const_vector_indices; | ||
if let Some(const_vector_i) = const_vector_indices { | ||
&const_vector_i.as_ref() | ||
} else { | ||
&Vec::<usize>::new() | ||
} | ||
} else { | ||
&Vec::<usize>::new() | ||
}; | ||
|
||
let mut copied_constant_arguments = vec![]; | ||
'make_args: for (i, arg) in first_args.iter().enumerate() { | ||
let mut op = self.codegen_operand(bx, &arg.node); | ||
let mut op = if const_vec_arg_indexes.contains(&i) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can just remove this In fact, should we maybe do this in |
||
if let mir::Operand::Constant(constant) = &arg.node | ||
&& constant.ty().is_simd() | ||
{ | ||
let (llval, ty) = self.early_evaluate_const_vector(bx, &constant); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So is there no LLVM intrinsic that needs a constant integer? It's only ever vectors? That seems odd. |
||
OperandRef { val: Immediate(llval), layout: bx.layout_of(ty) } | ||
} else { | ||
span_bug!(span, "argument at {i} must be a constant vector"); | ||
} | ||
} else { | ||
self.codegen_operand(bx, &arg.node) | ||
}; | ||
|
||
if let (0, Some(ty::InstanceKind::Virtual(_, idx))) = (i, def) { | ||
match op.val { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ use crate::traits::*; | |
use rustc_middle::mir; | ||
use rustc_middle::mir::interpret::ErrorHandled; | ||
use rustc_middle::ty::layout::HasTyCtxt; | ||
use rustc_middle::ty::ValTree; | ||
use rustc_middle::ty::{self, Ty}; | ||
use rustc_middle::{bug, span_bug}; | ||
use rustc_target::abi::Abi; | ||
|
@@ -29,7 +30,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |
.expect("erroneous constant missed by mono item collection") | ||
} | ||
|
||
/// This is a convenience helper for `simd_shuffle_indices`. It has the precondition | ||
/// This is a convenience helper for `early_evaluate_const_vector`. It has the precondition | ||
/// that the given `constant` is an `Const::Unevaluated` and must be convertible to | ||
/// a `ValTree`. If you want a more general version of this, talk to `wg-const-eval` on zulip. | ||
/// | ||
|
@@ -60,36 +61,48 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |
self.cx.tcx().const_eval_resolve_for_typeck(ty::ParamEnv::reveal_all(), uv, constant.span) | ||
} | ||
|
||
/// process constant containing SIMD shuffle indices | ||
pub fn simd_shuffle_indices( | ||
/// process constant SIMD vector or constant containing SIMD shuffle indices | ||
pub fn early_evaluate_const_vector( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find "early" here quite confusing, I don't think we otherwise use this adjective to refer to compile-time evaluation. Why not just |
||
&mut self, | ||
bx: &Bx, | ||
constant: &mir::ConstOperand<'tcx>, | ||
) -> (Bx::Value, Ty<'tcx>) { | ||
let ty = self.monomorphize(constant.ty()); | ||
let ty_is_simd = ty.is_simd(); | ||
let field_ty = if ty_is_simd { | ||
ty.simd_size_and_type(bx.tcx()).1 | ||
} else { | ||
ty.builtin_index().unwrap() | ||
}; | ||
let val = self | ||
.eval_unevaluated_mir_constant_to_valtree(constant) | ||
.ok() | ||
.map(|x| x.ok()) | ||
.flatten() | ||
.map(|val| { | ||
let field_ty = ty.builtin_index().unwrap(); | ||
let values: Vec<_> = val | ||
.unwrap_branch() | ||
.iter() | ||
.map(|field| { | ||
if let Some(prim) = field.try_to_scalar() { | ||
let layout = bx.layout_of(field_ty); | ||
let Abi::Scalar(scalar) = layout.abi else { | ||
bug!("from_const: invalid ByVal layout: {:#?}", layout); | ||
}; | ||
bx.scalar_to_backend(prim, scalar, bx.immediate_backend_type(layout)) | ||
} else { | ||
bug!("simd shuffle field {:?}", field) | ||
let mut values: Vec<Bx::Value> = Vec::new(); | ||
// For reliably being able to handle either: | ||
// pub struct defn(i32, i32) | ||
// call(1, 0); | ||
// OR | ||
// pub struct defn([i32, 2]) | ||
// call([1, 0]); | ||
// | ||
// And outputting: @call(<2 x i32> <i32 0, i32 1>) | ||
// thus treating them the same if they are a const vector | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand any part of this comment. What's But really, the biggest issue is that a nested loop makes no sense. There should be only one loop, the one that goes over all vector elements. The only difference between the two representations is that in one case we need one extra |
||
for field in val.unwrap_branch().iter() { | ||
match field { | ||
ValTree::Branch(_) => { | ||
let scalars = self.flatten_branch_to_scalars(bx, &field, field_ty); | ||
values.extend(scalars); | ||
} | ||
ValTree::Leaf(_) => { | ||
let scalar = self.extract_scalar(bx, &field, field_ty); | ||
values.push(scalar); | ||
} | ||
}) | ||
.collect(); | ||
bx.const_struct(&values, false) | ||
} | ||
} | ||
if ty_is_simd { bx.const_vector(&values) } else { bx.const_struct(&values, false) } | ||
}) | ||
.unwrap_or_else(|| { | ||
bx.tcx().dcx().emit_err(errors::ShuffleIndicesEvaluation { span: constant.span }); | ||
|
@@ -99,4 +112,34 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |
}); | ||
(val, ty) | ||
} | ||
|
||
fn flatten_branch_to_scalars( | ||
&mut self, | ||
bx: &Bx, | ||
branch: &ValTree<'tcx>, | ||
field_ty: Ty<'tcx>, | ||
) -> Vec<Bx::Value> { | ||
branch | ||
.unwrap_branch() | ||
.iter() | ||
.map(|field| match field { | ||
ValTree::Branch(_) => { | ||
bug!("Cannot have arbitrarily nested const vectors: {:#?}", field) | ||
} | ||
ValTree::Leaf(_) => self.extract_scalar(bx, field, field_ty), | ||
}) | ||
.collect() | ||
} | ||
|
||
fn extract_scalar(&mut self, bx: &Bx, field: &ValTree<'tcx>, field_ty: Ty<'tcx>) -> Bx::Value { | ||
if let Some(prim) = field.try_to_scalar() { | ||
let layout = bx.layout_of(field_ty); | ||
let Abi::Scalar(scalar) = layout.abi else { | ||
bug!("from_const: invalid ByVal layout: {:#?}", layout); | ||
}; | ||
bx.scalar_to_backend(prim, scalar, bx.immediate_backend_type(layout)) | ||
} else { | ||
bug!("field is not a scalar {:?}", field) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This diff is much bigger than needed. See #121225 for a PR that makes some other intrinsics require a constant argument; this should share that same check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact the even better alternative is probably to remove the simd_shuffle case entirely, and just add the new attribute to simd_shuffle.