Skip to content

Introduce __eq intrinsic #2100

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.

Sign up for GitHub

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

Merged
merged 16 commits into from
Jun 29, 2022
Merged

Introduce __eq intrinsic #2100

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
15a740c
Introduce __eq intrinsic
vaivaswatha Jun 24, 2022
0c9af62
Merge branch 'master' into vaivaswatha/intrinsic_eq
vaivaswatha Jun 24, 2022
9b154e1
Merge branch 'master' into vaivaswatha/intrinsic_eq
vaivaswatha Jun 25, 2022
ee35b5d
Add tests
vaivaswatha Jun 25, 2022
d10e152
Lower to Instruction::Cmp instead of to assembly in IRGen
vaivaswatha Jun 27, 2022
69173ac
Refactor intrinsics to all have arg and type arg vectors
vaivaswatha Jun 27, 2022
cc60855
Merge branch 'master' into vaivaswatha/intrinsic_eq
vaivaswatha Jun 27, 2022
3c8aa03
Fix cargo clippy
vaivaswatha Jun 27, 2022
b6a9b66
Address review comments
vaivaswatha Jun 28, 2022
fc96a5d
Add IR test for __eq
vaivaswatha Jun 28, 2022
1886acb
Merge branch 'master' into vaivaswatha/intrinsic_eq
vaivaswatha Jun 28, 2022
febfe3e
Add b256 and fix bool tests
vaivaswatha Jun 28, 2022
bf4db7a
Use nexln instead of check-next in FileCheck test
vaivaswatha Jun 29, 2022
0e50977
Use check instead of nextln which doesn't get it right
vaivaswatha Jun 29, 2022
54541f4
Merge branch 'master' into vaivaswatha/intrinsic_eq
vaivaswatha Jun 29, 2022
a675f18
Merge branch 'master' into vaivaswatha/intrinsic_eq
vaivaswatha Jun 29, 2022
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
64 changes: 15 additions & 49 deletions sway-core/src/control_flow_analysis/dead_code_analysis.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1029,66 +1029,32 @@ fn connect_expression(
}

fn connect_intrinsic_function(
kind: &TypedIntrinsicFunctionKind,
TypedIntrinsicFunctionKind {
kind, arguments, ..
}: &TypedIntrinsicFunctionKind,
graph: &mut ControlFlowGraph,
leaves: &[NodeIndex],
exit_node: Option<NodeIndex>,
tree_type: &TreeType,
) -> Result<Vec<NodeIndex>, CompileError> {
let result = match kind {
TypedIntrinsicFunctionKind::SizeOfVal { exp } => connect_expression(
let node = graph.add_node(format!("{}", kind).into());
Copy link
Contributor

@sezna sezna Jun 27, 2022

Choose a reason for hiding this comment

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

Suggested change
let node = graph.add_node(format!("{}", kind).into());
let node = graph.add_node(kind.to_string());

for leaf in leaves {
graph.add_edge(*leaf, node, "".into());
}
let mut result = vec![node];
let _ = arguments.iter().try_fold(&mut result, |accum, exp| {
let mut res = connect_expression(
&(*exp).expression,
graph,
leaves,
exit_node,
"size_of",
"intrinsic",
tree_type,
exp.span.clone(),
)?,
TypedIntrinsicFunctionKind::SizeOfType { .. } => {
let node = graph.add_node("size of type".into());
for leaf in leaves {
graph.add_edge(*leaf, node, "".into());
}
vec![node]
}
TypedIntrinsicFunctionKind::IsRefType { .. } => {
let node = graph.add_node("is ref type".into());
for leaf in leaves {
graph.add_edge(*leaf, node, "".into());
}
vec![node]
}
TypedIntrinsicFunctionKind::GetStorageKey => {
let node = graph.add_node("Get storage key".into());
for leaf in leaves {
graph.add_edge(*leaf, node, "".into());
}
vec![node]
}
TypedIntrinsicFunctionKind::Eq { lhs, rhs } => {
let mut lhsres = connect_expression(
&(*lhs).expression,
graph,
leaves,
exit_node,
"size_of",
tree_type,
rhs.span.clone(),
)?;
let mut rhsres = connect_expression(
&(*rhs).expression,
graph,
leaves,
exit_node,
"size_of",
tree_type,
rhs.span.clone(),
)?;
lhsres.append(&mut rhsres);
lhsres
}
};
)?;
accum.append(&mut res);
Ok::<_, CompileError>(accum)
})?;
Ok(result)
}

Expand Down
160 changes: 27 additions & 133 deletions sway-core/src/convert_parse_tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ use {
AbiDeclaration, AsmExpression, AsmOp, AsmRegister, AsmRegisterDeclaration, AstNode,
AstNodeContent, CallPath, CodeBlock, ConstantDeclaration, Declaration, EnumDeclaration,
EnumVariant, Expression, FunctionDeclaration, FunctionParameter, ImplSelf, ImplTrait,
ImportType, IncludeStatement, IntrinsicFunctionKind, LazyOp, Literal, MatchBranch,
MethodName, ParseTree, Purity, Reassignment, ReassignmentTarget, ReturnStatement,
Scrutinee, StorageDeclaration, StorageField, StructDeclaration, StructExpressionField,
StructField, StructScrutineeField, Supertrait, TraitDeclaration, TraitFn, TreeType,
TypeInfo, UseStatement, VariableDeclaration, Visibility, WhileLoop,
ImportType, IncludeStatement, LazyOp, Literal, MatchBranch, MethodName, ParseTree, Purity,
Reassignment, ReassignmentTarget, ReturnStatement, Scrutinee, StorageDeclaration,
StorageField, StructDeclaration, StructExpressionField, StructField, StructScrutineeField,
Supertrait, TraitDeclaration, TraitFn, TreeType, TypeInfo, UseStatement,
VariableDeclaration, Visibility, WhileLoop,
},
std::{
collections::HashMap,
Expand Down Expand Up @@ -102,16 +102,10 @@ pub enum ConvertParseTreeError {
GenericsNotSupportedHere { span: Span },
#[error("fully qualified paths are not supported here")]
FullyQualifiedPathsNotSupportedHere { span: Span },
#[error("__size_of does not take arguments")]
SizeOfTooManyArgs { span: Span },
#[error("__size_of requires exactly one generic argument")]
SizeOfOneGenericArg { span: Span },
#[error("__is_reference_type does not take arguments")]
IsReferenceTypeTooManyArgs { span: Span },
#[error("__is_reference_type requires exactly one generic argument")]
IsReferenceTypeOneGenericArg { span: Span },
#[error("__size_of_val requires exactly one argument")]
SizeOfValOneArg { span: Span },
#[error("__eq requires exactly two arguments")]
EqTwoArgs { span: Span },
#[error("tuple index out of range")]
Expand Down Expand Up @@ -201,11 +195,8 @@ impl Spanned for ConvertParseTreeError {
ConvertParseTreeError::FunctionArbitraryExpression { span } => span.clone(),
ConvertParseTreeError::GenericsNotSupportedHere { span } => span.clone(),
ConvertParseTreeError::FullyQualifiedPathsNotSupportedHere { span } => span.clone(),
ConvertParseTreeError::SizeOfTooManyArgs { span } => span.clone(),
ConvertParseTreeError::SizeOfOneGenericArg { span } => span.clone(),
ConvertParseTreeError::IsReferenceTypeTooManyArgs { span } => span.clone(),
ConvertParseTreeError::IsReferenceTypeOneGenericArg { span } => span.clone(),
ConvertParseTreeError::SizeOfValOneArg { span } => span.clone(),
ConvertParseTreeError::EqTwoArgs { span } => span.clone(),
ConvertParseTreeError::TupleIndexOutOfRange { span } => span.clone(),
ConvertParseTreeError::ShlNotImplemented { span } => span.clone(),
Expand Down Expand Up @@ -1445,134 +1436,37 @@ fn expr_to_expression(ec: &mut ErrorContext, expr: Expr) -> Result<Expression, E
}
}
None => {
if call_path.prefixes.is_empty()
&& !call_path.is_absolute
&& Intrinsic::try_from_str(call_path.suffix.as_str())
== Some(Intrinsic::SizeOf)
{
if !arguments.is_empty() {
let error = ConvertParseTreeError::SizeOfTooManyArgs { span };
return Err(ec.error(error));
}
let ty = match {
generics_opt.and_then(|(_double_colon_token, generic_args)| {
iter_to_array(generic_args.parameters.into_inner())
})
} {
Some([ty]) => ty,
None => {
let error = ConvertParseTreeError::SizeOfOneGenericArg { span };
return Err(ec.error(error));
}
};
let type_span = ty.span();
let type_name = ty_to_type_info(ec, ty)?;
Expression::IntrinsicFunction {
kind: IntrinsicFunctionKind::SizeOfType {
type_name,
type_span,
},
span,
}
} else if call_path.prefixes.is_empty()
&& !call_path.is_absolute
&& Intrinsic::try_from_str(call_path.suffix.as_str())
== Some(Intrinsic::GetStorageKey)
{
if !arguments.is_empty() {
let error = ConvertParseTreeError::GetStorageKeyTooManyArgs { span };
return Err(ec.error(error));
}
if generics_opt.is_some() {
let error = ConvertParseTreeError::GenericsNotSupportedHere { span };
return Err(ec.error(error));
}
Expression::IntrinsicFunction {
kind: IntrinsicFunctionKind::GetStorageKey,
span,
}
} else if call_path.prefixes.is_empty()
&& !call_path.is_absolute
&& Intrinsic::try_from_str(call_path.suffix.as_str())
== Some(Intrinsic::IsReferenceType)
{
if !arguments.is_empty() {
let error = ConvertParseTreeError::IsReferenceTypeTooManyArgs { span };
return Err(ec.error(error));
}
let ty = match {
generics_opt.and_then(|(_double_colon_token, generic_args)| {
iter_to_array(generic_args.parameters.into_inner())
})
} {
Some([ty]) => ty,
None => {
let error =
ConvertParseTreeError::IsReferenceTypeOneGenericArg { span };
return Err(ec.error(error));
}
};
let type_span = ty.span();
let type_name = ty_to_type_info(ec, ty)?;
Expression::IntrinsicFunction {
kind: IntrinsicFunctionKind::IsRefType {
type_name,
type_span,
},
span,
let type_arguments = match generics_opt {
Some((_double_colon_token, generic_args)) => {
generic_args_to_type_arguments(ec, generic_args)?
}
} else if call_path.prefixes.is_empty()
None => Vec::new(),
};
let intrinsic = Intrinsic::try_from_str(call_path.suffix.as_str());
if call_path.prefixes.is_empty()
&& !call_path.is_absolute
&& Intrinsic::try_from_str(call_path.suffix.as_str())
== Some(Intrinsic::SizeOfVal)
&& intrinsic.is_some()
{
let exp = match <[_; 1]>::try_from(arguments) {
Ok([exp]) => Box::new(exp),
Err(..) => {
let error = ConvertParseTreeError::SizeOfValOneArg { span };
return Err(ec.error(error));
}
};
Expression::IntrinsicFunction {
kind: IntrinsicFunctionKind::SizeOfVal { exp },
#[allow(clippy::unnecessary_unwrap)]
Copy link
Contributor

@otrho otrho Jun 28, 2022

Choose a reason for hiding this comment

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

This is weird, why is clippy complaining? intrinsic is an Option yeah?

kind: intrinsic.unwrap(),
arguments,
type_arguments,
span,
}
} else if call_path.prefixes.is_empty()
&& !call_path.is_absolute
&& Intrinsic::try_from_str(call_path.suffix.as_str()) == Some(Intrinsic::Eq)
{
let (lhs, rhs) = match <[_; 2]>::try_from(arguments) {
Ok([lhs, rhs]) => (Box::new(lhs), Box::new(rhs)),
Err(..) => {
let error = ConvertParseTreeError::EqTwoArgs { span };
return Err(ec.error(error));
}
};
Expression::IntrinsicFunction {
kind: IntrinsicFunctionKind::Eq { lhs, rhs },
} else if call_path.prefixes.is_empty() {
Expression::FunctionApplication {
name: call_path,
arguments,
type_arguments,
span,
}
} else {
let type_arguments = match generics_opt {
Some((_double_colon_token, generic_args)) => {
generic_args_to_type_arguments(ec, generic_args)?
}
None => Vec::new(),
};
if call_path.prefixes.is_empty() {
Expression::FunctionApplication {
name: call_path,
arguments,
type_arguments,
span,
}
} else {
Expression::DelineatedPath {
call_path,
args: arguments,
type_arguments,
span,
}
Expression::DelineatedPath {
call_path,
args: arguments,
type_arguments,
span,
}
}
}
Expand Down
20 changes: 17 additions & 3 deletions sway-core/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -995,8 +995,20 @@ pub enum CompileError {
NonConstantDeclValue { span: Span },
#[error("Declaring storage in a {program_kind} is not allowed.")]
StorageDeclarationInNonContract { program_kind: String, span: Span },
#[error("Unsupported argument type to intrinsic.")]
UnsupportedIntrinsicArgType { span: Span },
#[error("Unsupported argument type to {name}.")]
IntrinsicUnsupportedArgType { name: String, span: Span },
#[error("Call to {name} expectes {expected} arguments")]
IntrinsicIncorrectNumArgs {
name: String,
expected: u64,
span: Span,
},
#[error("Call to {name} expectes {expected} type arguments")]
IntrinsicIncorrectNumTArgs {
name: String,
expected: u64,
span: Span,
},
}

impl std::convert::From<TypeError> for CompileError {
Expand Down Expand Up @@ -1153,7 +1165,9 @@ impl Spanned for CompileError {
TupleIndexOutOfBounds { span, .. } => span.clone(),
NonConstantDeclValue { span } => span.clone(),
StorageDeclarationInNonContract { span, .. } => span.clone(),
UnsupportedIntrinsicArgType { span } => span.clone(),
IntrinsicUnsupportedArgType { span, .. } => span.clone(),
IntrinsicIncorrectNumArgs { span, .. } => span.clone(),
IntrinsicIncorrectNumTArgs { span, .. } => span.clone(),
}
}
}
Expand Down
37 changes: 26 additions & 11 deletions sway-core/src/ir_generation/function.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use super::{compile::compile_function, convert::*, lexical_map::LexicalMap, type

use fuel_crypto::Hasher;
use sway_ir::{Context, *};
use sway_parse::intrinsics::Intrinsic;
use sway_types::{
ident::Ident,
span::{Span, Spanned},
Expand Down Expand Up @@ -315,44 +316,58 @@ impl FnCompiler {
fn compile_intrinsic_function(
&mut self,
context: &mut Context,
kind: TypedIntrinsicFunctionKind,
TypedIntrinsicFunctionKind {
kind,
arguments,
type_arguments,
span: _,
}: TypedIntrinsicFunctionKind,
span: Span,
) -> Result<Value, CompileError> {
match kind {
TypedIntrinsicFunctionKind::SizeOfVal { exp } => {
Intrinsic::SizeOfVal => {
// We index the array with confidence that the type-checker did its job.
let exp = arguments[0].clone();
Copy link
Contributor

@sezna sezna Jun 27, 2022

Choose a reason for hiding this comment

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

Could you specify what you mean by "the type checker did its job"? The phrase "did its job" doesn't convey exactly what we are assuming here.

// Compile the expression in case of side-effects but ignore its value.
let ir_type = convert_resolved_typeid(context, &exp.return_type, &exp.span)?;
self.compile_expression(context, *exp)?;
self.compile_expression(context, exp)?;
Ok(Constant::get_uint(
context,
64,
ir_type_size_in_bytes(context, &ir_type),
None,
))
}
TypedIntrinsicFunctionKind::SizeOfType { type_id, type_span } => {
let ir_type = convert_resolved_typeid(context, &type_id, &type_span)?;
Intrinsic::SizeOfType => {
// We index the array with confidence that the type-checker did its job.
let targ = type_arguments[0].clone();
let ir_type = convert_resolved_typeid(context, &targ.type_id, &targ.span)?;
Ok(Constant::get_uint(
context,
64,
ir_type_size_in_bytes(context, &ir_type),
None,
))
}
TypedIntrinsicFunctionKind::IsRefType { type_id, type_span } => {
let ir_type = convert_resolved_typeid(context, &type_id, &type_span)?;
Intrinsic::IsReferenceType => {
// We index the array with confidence that the type-checker did its job.
let targ = type_arguments[0].clone();
let ir_type = convert_resolved_typeid(context, &targ.type_id, &targ.span)?;
Ok(Constant::get_bool(context, !ir_type.is_copy_type(), None))
}
TypedIntrinsicFunctionKind::GetStorageKey => {
Intrinsic::GetStorageKey => {
let span_md_idx = MetadataIndex::from_span(context, &span);
Ok(self
.current_block
.ins(context)
.get_storage_key(span_md_idx, None))
}
TypedIntrinsicFunctionKind::Eq { lhs, rhs } => {
let lhs_value = self.compile_expression(context, *lhs)?;
let rhs_value = self.compile_expression(context, *rhs)?;
Intrinsic::Eq => {
// We index the array with confidence that the type-checker did its job.
let lhs = arguments[0].clone();
let rhs = arguments[1].clone();
Comment on lines +369 to +370
Copy link
Contributor

@sezna sezna Jun 27, 2022

Choose a reason for hiding this comment

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

One thing we could do is codify the arguments into the enum itself, so we don't have a panic risk here.

e.g.

enum Intrinsic {
  Eq { lhs: TypedExpression, rhs: TypedExpression },
  SizeOf { type_argument: TypeInfo } 
}

etc...

I'm just suggesting this, but I wouldn't block this PR on it, as it could be a more major refactor. It would give us type safety here, though.

let lhs_value = self.compile_expression(context, lhs)?;
let rhs_value = self.compile_expression(context, rhs)?;
Ok(self.current_block.ins(context).cmp(
Predicate::Equal,
lhs_value,
Expand Down
Loading