Skip to content

Commit

Permalink
Remove redundant IR generation of constants and unused IR main inlini…
Browse files Browse the repository at this point in the history
…ng (#5998)

## Description

This PR:
- removes the redundant calls to `compile_declarations` function and the
function itself. The function was used in early versions of Sway, but
the way we compile items to IR has changed: functions are compiled at
call site, and structs and enums are inlined at instantiation site. In
the end, the function was only compiling constants, but that was already
done in the dedicated `compile_constants` calls.
- removes the `inline_main` optimization which was aggressively inlining
the whole program into `main` function. This was a necessity in early
versions of Sway and is not used anymore.

## Checklist

- [ ] I have linked to any relevant issues.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
  • Loading branch information
ironcev committed May 13, 2024
1 parent fa62d82 commit 8148817
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 148 deletions.
7 changes: 1 addition & 6 deletions sway-core/src/ir_generation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,13 @@ pub fn compile_program<'eng>(
};

match kind {
// predicates and scripts have the same codegen, their only difference is static
// Predicates and scripts have the same codegen, their only difference is static
// type-check time checks.
ty::TyProgramKind::Script { entry_function, .. } => compile::compile_script(
engines,
&mut ctx,
entry_function,
root.namespace.module(engines),
declarations,
&logged_types,
&messages_types,
&test_fns,
Expand All @@ -78,7 +77,6 @@ pub fn compile_program<'eng>(
&mut ctx,
entry_function,
root.namespace.module(engines),
declarations,
&logged_types,
&messages_types,
&test_fns,
Expand All @@ -101,15 +99,12 @@ pub fn compile_program<'eng>(
engines,
&mut ctx,
root.namespace.module(engines),
declarations,
&logged_types,
&messages_types,
&test_fns,
),
}?;

//println!("{ctx}");

ctx.verify().map_err(|ir_error: sway_ir::IrError| {
vec![CompileError::InternalOwned(
ir_error.to_string(),
Expand Down
110 changes: 0 additions & 110 deletions sway-core/src/ir_generation/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ pub(super) fn compile_script(
context: &mut Context,
entry_function: &DeclId<ty::TyFunctionDecl>,
namespace: &namespace::Module,
declarations: &[ty::TyDecl],
logged_types_map: &HashMap<TypeId, LogId>,
messages_types_map: &HashMap<TypeId, MessageId>,
test_fns: &[(Arc<ty::TyFunctionDecl>, DeclRefFunction)],
Expand All @@ -35,15 +34,6 @@ pub(super) fn compile_script(
let mut md_mgr = MetadataManager::default();

compile_constants(engines, context, &mut md_mgr, module, namespace).map_err(|err| vec![err])?;
compile_declarations(
engines,
context,
&mut md_mgr,
module,
namespace,
declarations,
)
.map_err(|err| vec![err])?;
compile_entry_function(
engines,
context,
Expand Down Expand Up @@ -73,7 +63,6 @@ pub(super) fn compile_predicate(
context: &mut Context,
entry_function: &DeclId<ty::TyFunctionDecl>,
namespace: &namespace::Module,
declarations: &[ty::TyDecl],
logged_types: &HashMap<TypeId, LogId>,
messages_types: &HashMap<TypeId, MessageId>,
test_fns: &[(Arc<ty::TyFunctionDecl>, DeclRefFunction)],
Expand All @@ -82,15 +71,6 @@ pub(super) fn compile_predicate(
let mut md_mgr = MetadataManager::default();

compile_constants(engines, context, &mut md_mgr, module, namespace).map_err(|err| vec![err])?;
compile_declarations(
engines,
context,
&mut md_mgr,
module,
namespace,
declarations,
)
.map_err(|err| vec![err])?;
compile_entry_function(
engines,
context,
Expand Down Expand Up @@ -130,15 +110,6 @@ pub(super) fn compile_contract(
let mut md_mgr = MetadataManager::default();

compile_constants(engines, context, &mut md_mgr, module, namespace).map_err(|err| vec![err])?;
compile_declarations(
engines,
context,
&mut md_mgr,
module,
namespace,
declarations,
)
.map_err(|err| vec![err])?;

if let Some(entry_function) = entry_function {
compile_entry_function(
Expand Down Expand Up @@ -202,7 +173,6 @@ pub(super) fn compile_library(
engines: &Engines,
context: &mut Context,
namespace: &namespace::Module,
declarations: &[ty::TyDecl],
logged_types_map: &HashMap<TypeId, LogId>,
messages_types_map: &HashMap<TypeId, MessageId>,
test_fns: &[(Arc<ty::TyFunctionDecl>, DeclRefFunction)],
Expand All @@ -211,15 +181,6 @@ pub(super) fn compile_library(
let mut md_mgr = MetadataManager::default();

compile_constants(engines, context, &mut md_mgr, module, namespace).map_err(|err| vec![err])?;
compile_declarations(
engines,
context,
&mut md_mgr,
module,
namespace,
declarations,
)
.map_err(|err| vec![err])?;
compile_tests(
engines,
context,
Expand Down Expand Up @@ -269,77 +230,6 @@ pub(crate) fn compile_constants(
Ok(())
}

// We don't really need to compile these declarations other than `const`s since:
// a) function decls are inlined into their call site and can be (re)created there, though ideally
// we'd give them their proper name by compiling them here.
// b) struct decls are also inlined at their instantiation site.
// c) ditto for enums.
//
// And for structs and enums in particular, we must ignore those with embedded generic types as
// they are monomorphised only at the instantiation site. We must ignore the generic declarations
// altogether anyway.
fn compile_declarations(
engines: &Engines,
context: &mut Context,
md_mgr: &mut MetadataManager,
module: Module,
namespace: &namespace::Module,
declarations: &[ty::TyDecl],
) -> Result<(), CompileError> {
for declaration in declarations {
match declaration {
ty::TyDecl::ConstantDecl(ty::ConstantDecl { decl_id, .. }) => {
let decl = engines.de().get_constant(decl_id);
let call_path = decl.call_path.clone();
compile_const_decl(
&mut LookupEnv {
engines,
context,
md_mgr,
module,
module_ns: Some(namespace),
function_compiler: None,
lookup: compile_const_decl,
},
&call_path,
&Some((*decl).clone()),
)?;
}

ty::TyDecl::FunctionDecl { .. } => {
// We no longer compile functions other than `main()` until we can improve the name
// resolution. Currently there isn't enough information in the AST to fully
// distinguish similarly named functions and especially trait methods.
//
//compile_function(context, module, decl).map(|_| ())?
}
ty::TyDecl::ImplTrait { .. } => {
// And for the same reason we don't need to compile impls at all.
//
// compile_impl(
// context,
// module,
// type_implementing_for,
// methods,
//)?,
}

ty::TyDecl::StructDecl { .. }
| ty::TyDecl::EnumDecl { .. }
| ty::TyDecl::EnumVariantDecl { .. }
| ty::TyDecl::TraitDecl { .. }
| ty::TyDecl::VariableDecl(_)
| ty::TyDecl::AbiDecl { .. }
| ty::TyDecl::GenericTypeForFunctionScope { .. }
| ty::TyDecl::StorageDecl { .. }
| ty::TyDecl::TypeAliasDecl { .. }
| ty::TyDecl::TraitTypeDecl { .. }
| ty::TyDecl::ErrorRecovery(..) => (),
}
}
Ok(())
}

#[allow(clippy::too_many_arguments)]
pub(super) fn compile_function(
engines: &Engines,
Expand Down
25 changes: 0 additions & 25 deletions sway-ir/src/optimize/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,6 @@ use crate::{
AnalysisResults, BlockArgument, Instruction, Module, Pass, PassMutability, ScopedPass,
};

pub const INLINE_MAIN_NAME: &str = "inline_main";

pub fn create_inline_in_main_pass() -> Pass {
Pass {
name: INLINE_MAIN_NAME,
descr: "inline from main fn.",
deps: vec![],
runner: ScopedPass::ModulePass(PassMutability::Transform(inline_in_main)),
}
}

pub const INLINE_MODULE_NAME: &str = "inline_module";

pub fn create_inline_in_module_pass() -> Pass {
Expand Down Expand Up @@ -146,20 +135,6 @@ pub fn inline_in_module(
Ok(modified)
}

pub fn inline_in_main(
context: &mut Context,
_: &AnalysisResults,
module: Module,
) -> Result<bool, IrError> {
// For now we inline everything into `main()`. Eventually we can be more selective.
for function in module.function_iter(context) {
if function.get_name(context) == "main" {
return inline_all_function_calls(context, &function);
}
}
Ok(false)
}

/// Inline all calls made from a specific function, effectively removing all `Call` instructions.
///
/// e.g., If this is applied to main() then all calls in the program are removed. This is
Expand Down
12 changes: 5 additions & 7 deletions sway-ir/src/pass_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ use crate::{
create_arg_demotion_pass, create_const_combine_pass, create_const_demotion_pass,
create_dce_pass, create_dom_fronts_pass, create_dominators_pass, create_escaped_symbols_pass,
create_fn_dedup_debug_profile_pass, create_fn_dedup_release_profile_pass, create_func_dce_pass,
create_inline_in_main_pass, create_inline_in_module_pass, create_mem2reg_pass,
create_memcpyopt_pass, create_misc_demotion_pass, create_module_printer_pass,
create_module_verifier_pass, create_postorder_pass, create_ret_demotion_pass,
create_simplify_cfg_pass, create_sroa_pass, Context, Function, IrError, Module,
CONSTCOMBINE_NAME, DCE_NAME, FNDEDUP_RELEASE_PROFILE_NAME, FUNC_DCE_NAME, INLINE_MODULE_NAME,
MEM2REG_NAME, SIMPLIFYCFG_NAME,
create_inline_in_module_pass, create_mem2reg_pass, create_memcpyopt_pass,
create_misc_demotion_pass, create_module_printer_pass, create_module_verifier_pass,
create_postorder_pass, create_ret_demotion_pass, create_simplify_cfg_pass, create_sroa_pass,
Context, Function, IrError, Module, CONSTCOMBINE_NAME, DCE_NAME, FNDEDUP_RELEASE_PROFILE_NAME,
FUNC_DCE_NAME, INLINE_MODULE_NAME, MEM2REG_NAME, SIMPLIFYCFG_NAME,
};
use downcast_rs::{impl_downcast, Downcast};
use rustc_hash::FxHashMap;
Expand Down Expand Up @@ -311,7 +310,6 @@ pub fn register_known_passes(pm: &mut PassManager) {
pm.register(create_mem2reg_pass());
pm.register(create_sroa_pass());
pm.register(create_inline_in_module_pass());
pm.register(create_inline_in_main_pass());
pm.register(create_const_combine_pass());
pm.register(create_simplify_cfg_pass());
pm.register(create_func_dce_pass());
Expand Down

0 comments on commit 8148817

Please sign in to comment.