From f92c3f249bb5acb8ecb00a3a0118b71280bfaebd Mon Sep 17 00:00:00 2001 From: Vaivaswatha N Date: Tue, 28 Oct 2025 18:05:55 +0530 Subject: [PATCH] SROA: Do not generate GEPs when direct address is available (#7457) Fixes #7433 --- .github/workflows/ci.yml | 2 +- forc-pkg/src/manifest/build_profile.rs | 19 +++--- forc-pkg/src/pkg.rs | 8 ++- forc-plugins/forc-client/src/cmd/deploy.rs | 3 + forc-plugins/forc-client/src/cmd/run.rs | 3 + forc-plugins/forc-client/src/op/deploy.rs | 7 ++- forc-plugins/forc-client/src/op/run/mod.rs | 7 ++- forc-test/src/lib.rs | 5 +- forc/src/cli/commands/contract_id.rs | 4 +- forc/src/cli/commands/predicate_root.rs | 4 +- forc/src/cli/commands/test.rs | 15 +++-- forc/src/cli/shared.rs | 38 ++++++++---- forc/src/ops/forc_build.rs | 14 +++-- forc/src/ops/forc_contract_id.rs | 8 ++- forc/src/ops/forc_predicate_root.rs | 8 ++- sway-core/src/build_config.rs | 40 +++++++++--- sway-core/src/ir_generation.rs | 3 +- sway-core/src/lib.rs | 34 +++++----- sway-ir/src/optimize/sroa.rs | 72 +++++++++++++--------- sway-ir/src/parser.rs | 4 +- sway-ir/src/pass_manager.rs | 31 +++++++++- sway-ir/src/verify.rs | 4 +- sway-ir/tests/tests.rs | 2 +- test/src/e2e_vm_tests/harness.rs | 1 + test/src/ir_generation/mod.rs | 4 +- test/src/main.rs | 25 ++++++-- 26 files changed, 255 insertions(+), 110 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 37b8efca6d..0566a07592 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -432,7 +432,7 @@ jobs: run: | fuel-core run --db-type in-memory --debug & sleep 5 && - cargo run --locked --release --bin test -- --locked --release + cargo run --locked --release --bin test -- --locked --release --verify-ir all # TODO: Remove this upon merging std tests with the rest of the E2E tests. cargo-test-lib-std: diff --git a/forc-pkg/src/manifest/build_profile.rs b/forc-pkg/src/manifest/build_profile.rs index d186c6b984..79d01ad4fd 100644 --- a/forc-pkg/src/manifest/build_profile.rs +++ b/forc-pkg/src/manifest/build_profile.rs @@ -1,5 +1,5 @@ use serde::{Deserialize, Serialize}; -use sway_core::{Backtrace, OptLevel, PrintAsm, PrintIr}; +use sway_core::{Backtrace, IrCli, OptLevel, PrintAsm}; use crate::DumpOpts; @@ -15,7 +15,9 @@ pub struct BuildProfile { pub print_dca_graph_url_format: Option, pub dump: DumpOpts, #[serde(default)] - pub print_ir: PrintIr, + pub print_ir: IrCli, + #[serde(default)] + pub verify_ir: IrCli, #[serde(default)] pub print_asm: PrintAsm, #[serde(default)] @@ -54,7 +56,8 @@ impl BuildProfile { print_ast: false, print_dca_graph: None, print_dca_graph_url_format: None, - print_ir: PrintIr::default(), + print_ir: IrCli::default(), + verify_ir: IrCli::default(), print_asm: PrintAsm::default(), print_bytecode: false, print_bytecode_spans: false, @@ -77,7 +80,8 @@ impl BuildProfile { print_ast: false, print_dca_graph: None, print_dca_graph_url_format: None, - print_ir: PrintIr::default(), + print_ir: IrCli::default(), + verify_ir: IrCli::default(), print_asm: PrintAsm::default(), print_bytecode: false, print_bytecode_spans: false, @@ -107,7 +111,7 @@ impl Default for BuildProfile { #[cfg(test)] mod tests { use crate::{BuildProfile, DumpOpts, PackageManifest}; - use sway_core::{Backtrace, OptLevel, PrintAsm, PrintIr}; + use sway_core::{Backtrace, IrCli, OptLevel, PrintAsm}; #[test] fn test_build_profiles() { @@ -132,7 +136,7 @@ mod tests { // Profile based on debug profile with adjusted IR printing options. let expected = BuildProfile { name: "".into(), - print_ir: PrintIr { + print_ir: IrCli { initial: true, r#final: false, modified_only: true, @@ -163,7 +167,8 @@ mod tests { print_ast: true, print_dca_graph: Some("dca_graph".into()), print_dca_graph_url_format: Some("print_dca_graph_url_format".into()), - print_ir: PrintIr::r#final(), + print_ir: IrCli::r#final(), + verify_ir: IrCli::none(), print_asm: PrintAsm::all(), print_bytecode: true, print_bytecode_spans: false, diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index fedda7e9ba..48a3827a47 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -47,7 +47,7 @@ use sway_core::{ write_dwarf, BuildTarget, Engines, FinalizedEntry, LspConfig, }; use sway_core::{namespace::Package, Observer}; -use sway_core::{set_bytecode_configurables_offset, DbgGeneration, PrintAsm, PrintIr}; +use sway_core::{set_bytecode_configurables_offset, DbgGeneration, IrCli, PrintAsm}; use sway_error::{error::CompileError, handler::Handler, warning::CompileWarning}; use sway_features::ExperimentalFeatures; use sway_types::{Ident, ProgramId, Span, Spanned}; @@ -261,7 +261,7 @@ pub struct PrintOpts { /// Print the original source code together with bytecode. pub bytecode_spans: bool, /// Print the generated Sway IR (Intermediate Representation). - pub ir: PrintIr, + pub ir: IrCli, /// Output build errors and warnings in reverse order. pub reverse_order: bool, } @@ -290,6 +290,7 @@ pub struct DumpOpts { pub struct BuildOpts { pub pkg: PkgOpts, pub print: PrintOpts, + pub verify_ir: IrCli, pub minify: MinifyOpts, pub dump: DumpOpts, /// If set, generates a JSON file containing the hex-encoded script binary. @@ -1581,6 +1582,7 @@ pub fn sway_build_config( build_profile.print_bytecode_spans, ) .with_print_ir(build_profile.print_ir.clone()) + .with_verify_ir(build_profile.verify_ir.clone()) .with_include_tests(build_profile.include_tests) .with_time_phases(build_profile.time_phases) .with_profile(build_profile.profile) @@ -2119,6 +2121,7 @@ fn build_profile_from_opts( let BuildOpts { pkg, print, + verify_ir, time_phases, profile: profile_opt, build_profile, @@ -2158,6 +2161,7 @@ fn build_profile_from_opts( .clone_from(&print.dca_graph_url_format); } profile.print_ir |= print.ir.clone(); + profile.verify_ir |= verify_ir.clone(); profile.print_asm |= print.asm; profile.print_bytecode |= print.bytecode; profile.print_bytecode_spans |= print.bytecode_spans; diff --git a/forc-plugins/forc-client/src/cmd/deploy.rs b/forc-plugins/forc-client/src/cmd/deploy.rs index a30a40b3b0..8796531f31 100644 --- a/forc-plugins/forc-client/src/cmd/deploy.rs +++ b/forc-plugins/forc-client/src/cmd/deploy.rs @@ -1,5 +1,6 @@ use crate::NodeTarget; use clap::Parser; +use forc::cli::shared::IrCliOpt; pub use forc::cli::shared::{BuildOutput, Minify, Pkg, Print}; use forc_pkg::BuildProfile; pub use forc_tx::{Gas, Maturity}; @@ -23,6 +24,8 @@ pub struct Command { pub minify: Minify, #[clap(flatten)] pub print: Print, + #[arg(long, value_parser = clap::builder::PossibleValuesParser::new(IrCliOpt::cli_options()))] + pub verify_ir: Option>, #[clap(flatten)] pub gas: Gas, #[clap(flatten)] diff --git a/forc-plugins/forc-client/src/cmd/run.rs b/forc-plugins/forc-client/src/cmd/run.rs index 7168f69c9a..564cffa7b1 100644 --- a/forc-plugins/forc-client/src/cmd/run.rs +++ b/forc-plugins/forc-client/src/cmd/run.rs @@ -1,5 +1,6 @@ use crate::NodeTarget; use clap::Parser; +use forc::cli::shared::IrCliOpt; use fuel_crypto::SecretKey; pub use super::submit::Network; @@ -15,6 +16,8 @@ pub struct Command { pub pkg: Pkg, #[clap(flatten)] pub minify: Minify, + #[arg(long, value_parser = clap::builder::PossibleValuesParser::new(IrCliOpt::cli_options()))] + pub verify_ir: Option>, #[clap(flatten)] pub print: Print, #[clap(flatten)] diff --git a/forc-plugins/forc-client/src/op/deploy.rs b/forc-plugins/forc-client/src/op/deploy.rs index 9638ff208c..d4cd62451f 100644 --- a/forc-plugins/forc-client/src/op/deploy.rs +++ b/forc-plugins/forc-client/src/op/deploy.rs @@ -12,6 +12,7 @@ use crate::{ }, }; use anyhow::{bail, Context, Result}; +use forc::cli::shared::IrCliOpt; use forc_pkg::{self as pkg, DumpOpts, PackageManifestFile}; use forc_pkg::{manifest::GenericManifestFile, MemberFilter}; use forc_tracing::{println_action_green, println_warning}; @@ -43,7 +44,7 @@ use std::{ sync::Arc, time::Duration, }; -use sway_core::{asm_generation::ProgramABI, language::parsed::TreeType, BuildTarget}; +use sway_core::{asm_generation::ProgramABI, language::parsed::TreeType, BuildTarget, IrCli}; /// Default maximum contract size allowed for a single contract. If the target /// contract size is bigger than this amount, forc-deploy will automatically @@ -848,6 +849,10 @@ fn build_opts_from_cmd(cmd: &cmd::Deploy, member_filter: pkg::MemberFilter) -> p ir: cmd.print.ir(), reverse_order: cmd.print.reverse_order, }, + verify_ir: cmd + .verify_ir + .as_ref() + .map_or(IrCli::default(), |opts| IrCliOpt::from(opts).0), dump: DumpOpts::default(), time_phases: cmd.print.time_phases, profile: cmd.print.profile, diff --git a/forc-plugins/forc-client/src/op/run/mod.rs b/forc-plugins/forc-client/src/op/run/mod.rs index 695e180e50..cbbc24f949 100644 --- a/forc-plugins/forc-client/src/op/run/mod.rs +++ b/forc-plugins/forc-client/src/op/run/mod.rs @@ -8,6 +8,7 @@ use crate::{ }, }; use anyhow::{anyhow, bail, Context, Result}; +use forc::cli::shared::IrCliOpt; use forc_pkg::{self as pkg, fuel_core_not_running, DumpOpts, PackageManifestFile}; use forc_tracing::println_warning; use forc_util::tx_utils::format_log_receipts; @@ -25,8 +26,8 @@ use fuels_accounts::{provider::Provider, Account, ViewOnlyAccount}; use pkg::BuiltPackage; use std::time::Duration; use std::{path::PathBuf, str::FromStr}; -use sway_core::language::parsed::TreeType; use sway_core::BuildTarget; +use sway_core::{language::parsed::TreeType, IrCli}; use tokio::time::timeout; use tracing::info; @@ -320,6 +321,10 @@ fn build_opts_from_cmd(cmd: &cmd::Run) -> pkg::BuildOpts { ir: cmd.print.ir(), reverse_order: cmd.print.reverse_order, }, + verify_ir: cmd + .verify_ir + .as_ref() + .map_or(IrCli::default(), |opts| IrCliOpt::from(opts).0), minify: pkg::MinifyOpts { json_abi: cmd.minify.json_abi, json_storage_slots: cmd.minify.json_storage_slots, diff --git a/forc-test/src/lib.rs b/forc-test/src/lib.rs index 535248a864..2e8f3a3883 100644 --- a/forc-test/src/lib.rs +++ b/forc-test/src/lib.rs @@ -19,7 +19,7 @@ use pkg::{Built, BuiltPackage}; use rand::{Rng, SeedableRng}; use rayon::prelude::*; use std::{collections::HashMap, fs, path::PathBuf, sync::Arc}; -use sway_core::BuildTarget; +use sway_core::{BuildTarget, IrCli}; use sway_types::Span; use tx::consensus_parameters::ConsensusParametersV1; use tx::{ConsensusParameters, ContractParameters, ScriptParameters, TxParameters}; @@ -136,6 +136,7 @@ pub enum PackageWithDeploymentToTest { pub struct TestOpts { pub pkg: pkg::PkgOpts, pub print: pkg::PrintOpts, + pub verify_ir: IrCli, pub minify: pkg::MinifyOpts, /// If set, outputs a binary file representing the script bytes. pub binary_outfile: Option, @@ -454,6 +455,7 @@ impl From for pkg::BuildOpts { pkg::BuildOpts { pkg: val.pkg, print: val.print, + verify_ir: val.verify_ir, minify: val.minify, dump: DumpOpts::default(), binary_outfile: val.binary_outfile, @@ -481,6 +483,7 @@ impl TestOpts { pkg::BuildOpts { pkg: self.pkg, print: self.print, + verify_ir: self.verify_ir, minify: self.minify, dump: DumpOpts::default(), binary_outfile: self.binary_outfile, diff --git a/forc/src/cli/commands/contract_id.rs b/forc/src/cli/commands/contract_id.rs index 8564093794..fff5a4f78c 100644 --- a/forc/src/cli/commands/contract_id.rs +++ b/forc/src/cli/commands/contract_id.rs @@ -1,5 +1,5 @@ use crate::{ - cli::shared::{BuildOutput, BuildProfile, Minify, Pkg, Print}, + cli::shared::{BuildOutput, BuildProfile, IrCliOpt, Minify, Pkg, Print}, ops::forc_contract_id, }; use clap::Parser; @@ -22,6 +22,8 @@ pub struct Command { pub minify: Minify, #[clap(flatten)] pub print: Print, + #[arg(long, value_parser = clap::builder::PossibleValuesParser::new(IrCliOpt::cli_options()))] + pub verify_ir: Option>, #[clap(flatten)] pub build_output: BuildOutput, #[clap(flatten)] diff --git a/forc/src/cli/commands/predicate_root.rs b/forc/src/cli/commands/predicate_root.rs index 676c85e998..6deedbc7fb 100644 --- a/forc/src/cli/commands/predicate_root.rs +++ b/forc/src/cli/commands/predicate_root.rs @@ -2,7 +2,7 @@ use clap::Parser; use forc_util::ForcResult; pub use crate::cli::shared::{BuildOutput, BuildProfile, Minify, Pkg, Print}; -use crate::ops::forc_predicate_root; +use crate::{cli::shared::IrCliOpt, ops::forc_predicate_root}; forc_util::cli_examples! { crate::cli::Opt { @@ -21,6 +21,8 @@ pub struct Command { pub minify: Minify, #[clap(flatten)] pub print: Print, + #[arg(long, value_parser = clap::builder::PossibleValuesParser::new(IrCliOpt::cli_options()))] + pub verify_ir: Option>, #[clap(flatten)] pub build_output: BuildOutput, #[clap(flatten)] diff --git a/forc/src/cli/commands/test.rs b/forc/src/cli/commands/test.rs index 607c797c43..ef19b80313 100644 --- a/forc/src/cli/commands/test.rs +++ b/forc/src/cli/commands/test.rs @@ -1,4 +1,4 @@ -use crate::cli; +use crate::cli::{self, shared::IrCliOpt}; use ansiterm::Colour; use clap::Parser; use forc_pkg as pkg; @@ -9,7 +9,7 @@ use forc_util::{ ForcError, ForcResult, }; use fuel_abi_types::{abi::program::PanickingCall, revert_info::RevertKind}; -use sway_core::{asm_generation::ProgramABI, fuel_prelude::fuel_tx::Receipt}; +use sway_core::{asm_generation::ProgramABI, fuel_prelude::fuel_tx::Receipt, IrCli}; use tracing::info; forc_util::cli_examples! { @@ -337,12 +337,12 @@ fn print_test_output( fn opts_from_cmd(cmd: Command) -> forc_test::TestOpts { forc_test::TestOpts { pkg: pkg::PkgOpts { - path: cmd.build.pkg.path, + path: cmd.build.pkg.path.clone(), offline: cmd.build.pkg.offline, terse: cmd.build.pkg.terse, locked: cmd.build.pkg.locked, - output_directory: cmd.build.pkg.output_directory, - ipfs_node: cmd.build.pkg.ipfs_node.unwrap_or_default(), + output_directory: cmd.build.pkg.output_directory.clone(), + ipfs_node: cmd.build.pkg.ipfs_node.clone().unwrap_or_default(), }, print: pkg::PrintOpts { ast: cmd.build.print.ast, @@ -354,6 +354,11 @@ fn opts_from_cmd(cmd: Command) -> forc_test::TestOpts { ir: cmd.build.print.ir(), reverse_order: cmd.build.print.reverse_order, }, + verify_ir: cmd + .build + .verify_ir + .as_ref() + .map_or(IrCli::default(), |opts| IrCliOpt::from(opts).0), time_phases: cmd.build.print.time_phases, profile: cmd.build.print.profile, metrics_outfile: cmd.build.print.metrics_outfile, diff --git a/forc/src/cli/shared.rs b/forc/src/cli/shared.rs index 75081ec328..cb2c77c15f 100644 --- a/forc/src/cli/shared.rs +++ b/forc/src/cli/shared.rs @@ -1,7 +1,7 @@ //! Sets of arguments that are shared between commands. use clap::{ArgGroup, Args, Parser}; use forc_pkg::source::IPFSNode; -use sway_core::{BuildTarget, PrintAsm, PrintIr}; +use sway_core::{BuildTarget, IrCli, PrintAsm}; use sway_ir::PassManager; #[derive(Debug, Args)] @@ -82,6 +82,16 @@ pub struct Build { pub pkg: Pkg, #[clap(flatten)] pub print: Print, + /// Verify the generated Sway IR (Intermediate Representation). + /// + /// Values that can be combined: + /// - initial: initial IR prior to any optimization passes. + /// - final: final IR after applying all optimization passes. + /// - : the name of an optimization pass. Verifies the IR state after that pass. + /// - all: short for initial, final, and all the optimization passes. + /// - modified: verify a requested optimization pass only if it has modified the IR. + #[arg(long, verbatim_doc_comment, num_args(1..=IrCliOpt::max_num_args()), value_parser = clap::builder::PossibleValuesParser::new(IrCliOpt::cli_options()))] + pub verify_ir: Option>, #[clap(flatten)] pub minify: Minify, #[clap(flatten)] @@ -163,7 +173,7 @@ pub struct Print { /// - abstract: short for both virtual and allocated ASM. /// - final: final ASM that gets serialized to the target VM bytecode. /// - all: short for virtual, allocated, and final ASM. - #[arg(long, verbatim_doc_comment, num_args(1..=5), value_parser = clap::builder::PossibleValuesParser::new(&PrintAsmCliOpt::CLI_OPTIONS))] + #[arg(long, verbatim_doc_comment, num_args(1..=PrintAsmCliOpt::CLI_OPTIONS.len()), value_parser = clap::builder::PossibleValuesParser::new(&PrintAsmCliOpt::CLI_OPTIONS))] pub asm: Option>, /// Print the bytecode. /// @@ -178,7 +188,7 @@ pub struct Print { /// - : the name of an optimization pass. Prints the IR state after that pass. /// - all: short for initial, final, and all the optimization passes. /// - modified: print a requested optimization pass only if it has modified the IR. - #[arg(long, verbatim_doc_comment, num_args(1..=18), value_parser = clap::builder::PossibleValuesParser::new(PrintIrCliOpt::cli_options()))] + #[arg(long, verbatim_doc_comment, num_args(1..=IrCliOpt::max_num_args()), value_parser = clap::builder::PossibleValuesParser::new(IrCliOpt::cli_options()))] pub ir: Option>, /// Output the time elapsed over each part of the compilation process. #[clap(long)] @@ -201,10 +211,10 @@ impl Print { .map_or(PrintAsm::default(), |opts| PrintAsmCliOpt::from(opts).0) } - pub fn ir(&self) -> PrintIr { + pub fn ir(&self) -> IrCli { self.ir .as_ref() - .map_or(PrintIr::default(), |opts| PrintIrCliOpt::from(opts).0) + .map_or(IrCli::default(), |opts| IrCliOpt::from(opts).0) } } @@ -296,9 +306,9 @@ impl From<&Vec> for PrintAsmCliOpt { } } -pub struct PrintIrCliOpt(pub PrintIr); +pub struct IrCliOpt(pub IrCli); -impl PrintIrCliOpt { +impl IrCliOpt { const INITIAL: &'static str = "initial"; const FINAL: &'static str = "final"; const ALL: &'static str = "all"; @@ -313,16 +323,20 @@ impl PrintIrCliOpt { .cloned() .collect() } + + pub fn max_num_args() -> usize { + Self::CLI_OPTIONS.len() + PassManager::OPTIMIZATION_PASSES.len() + } } -impl From<&Vec> for PrintIrCliOpt { +impl From<&Vec> for IrCliOpt { fn from(value: &Vec) -> Self { let contains_opt = |opt: &str| value.iter().any(|val| *val == opt); - let print_ir = if contains_opt(Self::ALL) { - PrintIr::all(contains_opt(Self::MODIFIED)) + let ir_cli = if contains_opt(Self::ALL) { + IrCli::all(contains_opt(Self::MODIFIED)) } else { - PrintIr { + IrCli { initial: contains_opt(Self::INITIAL), r#final: contains_opt(Self::FINAL), modified_only: contains_opt(Self::MODIFIED), @@ -334,6 +348,6 @@ impl From<&Vec> for PrintIrCliOpt { } }; - Self(print_ir) + Self(ir_cli) } } diff --git a/forc/src/ops/forc_build.rs b/forc/src/ops/forc_build.rs index 390876a265..97b5062f3f 100644 --- a/forc/src/ops/forc_build.rs +++ b/forc/src/ops/forc_build.rs @@ -1,7 +1,8 @@ -use crate::cli::BuildCommand; +use crate::cli::{shared::IrCliOpt, BuildCommand}; use forc_pkg as pkg; use forc_util::ForcResult; use pkg::MemberFilter; +use sway_core::IrCli; pub fn build(cmd: BuildCommand) -> ForcResult { let opts = opts_from_cmd(cmd); @@ -12,12 +13,12 @@ pub fn build(cmd: BuildCommand) -> ForcResult { fn opts_from_cmd(cmd: BuildCommand) -> pkg::BuildOpts { pkg::BuildOpts { pkg: pkg::PkgOpts { - path: cmd.build.pkg.path, + path: cmd.build.pkg.path.clone(), offline: cmd.build.pkg.offline, terse: cmd.build.pkg.terse, locked: cmd.build.pkg.locked, - output_directory: cmd.build.pkg.output_directory, - ipfs_node: cmd.build.pkg.ipfs_node.unwrap_or_default(), + output_directory: cmd.build.pkg.output_directory.clone(), + ipfs_node: cmd.build.pkg.ipfs_node.clone().unwrap_or_default(), }, print: pkg::PrintOpts { ast: cmd.build.print.ast, @@ -29,6 +30,11 @@ fn opts_from_cmd(cmd: BuildCommand) -> pkg::BuildOpts { ir: cmd.build.print.ir(), reverse_order: cmd.build.print.reverse_order, }, + verify_ir: cmd + .build + .verify_ir + .as_ref() + .map_or(IrCli::default(), |opts| IrCliOpt::from(opts).0), dump: pkg::DumpOpts { dump_impls: cmd.build.dump.dump_impls, }, diff --git a/forc/src/ops/forc_contract_id.rs b/forc/src/ops/forc_contract_id.rs index 47203207c5..044f0325d7 100644 --- a/forc/src/ops/forc_contract_id.rs +++ b/forc/src/ops/forc_contract_id.rs @@ -1,8 +1,8 @@ -use crate::cli::ContractIdCommand; +use crate::cli::{shared::IrCliOpt, ContractIdCommand}; use anyhow::{bail, Result}; use forc_pkg::{self as pkg, build_with_options, DumpOpts}; use forc_tracing::println_green; -use sway_core::{fuel_prelude::fuel_tx, BuildTarget}; +use sway_core::{fuel_prelude::fuel_tx, BuildTarget, IrCli}; use tracing::info; pub fn contract_id(command: ContractIdCommand) -> Result<()> { @@ -63,6 +63,10 @@ fn build_opts_from_cmd(cmd: &ContractIdCommand) -> pkg::BuildOpts { ir: cmd.print.ir(), reverse_order: cmd.print.reverse_order, }, + verify_ir: cmd + .verify_ir + .as_ref() + .map_or(IrCli::default(), |opts| IrCliOpt::from(opts).0), dump: DumpOpts::default(), time_phases: cmd.print.time_phases, profile: cmd.print.profile, diff --git a/forc/src/ops/forc_predicate_root.rs b/forc/src/ops/forc_predicate_root.rs index c62c1cc099..1e544bc430 100644 --- a/forc/src/ops/forc_predicate_root.rs +++ b/forc/src/ops/forc_predicate_root.rs @@ -1,7 +1,7 @@ -use crate::cli::PredicateRootCommand; +use crate::cli::{shared::IrCliOpt, PredicateRootCommand}; use anyhow::Result; use forc_pkg::{self as pkg, build_with_options, DumpOpts}; -use sway_core::BuildTarget; +use sway_core::{BuildTarget, IrCli}; pub fn predicate_root(command: PredicateRootCommand) -> Result<()> { let build_options = build_opts_from_cmd(command); @@ -32,6 +32,10 @@ fn build_opts_from_cmd(cmd: PredicateRootCommand) -> pkg::BuildOpts { ir: cmd.print.ir(), reverse_order: cmd.print.reverse_order, }, + verify_ir: cmd + .verify_ir + .as_ref() + .map_or(IrCli::default(), |opts| IrCliOpt::from(opts).0), dump: DumpOpts::default(), time_phases: cmd.print.time_phases, profile: cmd.print.profile, diff --git a/sway-core/src/build_config.rs b/sway-core/src/build_config.rs index 80d7f1a651..0e143e23d5 100644 --- a/sway-core/src/build_config.rs +++ b/sway-core/src/build_config.rs @@ -6,7 +6,7 @@ use std::{ sync::Arc, }; use strum::{Display, EnumString}; -use sway_ir::{PassManager, PrintPassesOpts}; +use sway_ir::{PassManager, PrintPassesOpts, VerifyPassesOpts}; #[derive( Clone, @@ -118,7 +118,7 @@ impl std::ops::BitOrAssign for PrintAsm { /// Which IR states to print. #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] -pub struct PrintIr { +pub struct IrCli { pub initial: bool, pub r#final: bool, #[serde(rename = "modified")] @@ -126,7 +126,7 @@ pub struct PrintIr { pub passes: Vec, } -impl Default for PrintIr { +impl Default for IrCli { fn default() -> Self { Self { initial: false, @@ -137,7 +137,7 @@ impl Default for PrintIr { } } -impl PrintIr { +impl IrCli { pub fn all(modified_only: bool) -> Self { Self { initial: true, @@ -162,7 +162,7 @@ impl PrintIr { } } -impl std::ops::BitOrAssign for PrintIr { +impl std::ops::BitOrAssign for IrCli { fn bitor_assign(&mut self, rhs: Self) { self.initial |= rhs.initial; self.r#final |= rhs.r#final; @@ -179,8 +179,19 @@ impl std::ops::BitOrAssign for PrintIr { } } -impl From<&PrintIr> for PrintPassesOpts { - fn from(value: &PrintIr) -> Self { +impl From<&IrCli> for PrintPassesOpts { + fn from(value: &IrCli) -> Self { + Self { + initial: value.initial, + r#final: value.r#final, + modified_only: value.modified_only, + passes: HashSet::from_iter(value.passes.iter().cloned()), + } + } +} + +impl From<&IrCli> for VerifyPassesOpts { + fn from(value: &IrCli) -> Self { Self { initial: value.initial, r#final: value.r#final, @@ -225,7 +236,8 @@ pub struct BuildConfig { pub(crate) print_asm: PrintAsm, pub(crate) print_bytecode: bool, pub(crate) print_bytecode_spans: bool, - pub(crate) print_ir: PrintIr, + pub(crate) print_ir: IrCli, + pub(crate) verify_ir: IrCli, pub(crate) include_tests: bool, pub(crate) optimization_level: OptLevel, pub(crate) backtrace: Backtrace, @@ -276,7 +288,8 @@ impl BuildConfig { print_asm: PrintAsm::default(), print_bytecode: false, print_bytecode_spans: false, - print_ir: PrintIr::default(), + print_ir: IrCli::default(), + verify_ir: IrCli::default(), include_tests: false, time_phases: false, profile: false, @@ -324,13 +337,20 @@ impl BuildConfig { } } - pub fn with_print_ir(self, a: PrintIr) -> Self { + pub fn with_print_ir(self, a: IrCli) -> Self { Self { print_ir: a, ..self } } + pub fn with_verify_ir(self, a: IrCli) -> Self { + Self { + verify_ir: a, + ..self + } + } + pub fn with_time_phases(self, a: bool) -> Self { Self { time_phases: a, diff --git a/sway-core/src/ir_generation.rs b/sway-core/src/ir_generation.rs index af04300cbb..b8687d0d47 100644 --- a/sway-core/src/ir_generation.rs +++ b/sway-core/src/ir_generation.rs @@ -440,7 +440,8 @@ pub fn compile_program<'a>( ir_error.to_string(), Span::dummy(), )] - }) + })?; + Ok(ctx) } fn type_correction(ctx: &mut Context) -> Result<(), IrError> { diff --git a/sway-core/src/lib.rs b/sway-core/src/lib.rs index e22ae16d35..72e7e56949 100644 --- a/sway-core/src/lib.rs +++ b/sway-core/src/lib.rs @@ -36,9 +36,7 @@ pub use asm_generation::from_ir::compile_ir_context_to_finalized_asm; use asm_generation::FinalizedAsm; pub use asm_generation::{CompiledBytecode, FinalizedEntry}; pub use build_config::DbgGeneration; -pub use build_config::{ - Backtrace, BuildConfig, BuildTarget, LspConfig, OptLevel, PrintAsm, PrintIr, -}; +pub use build_config::{Backtrace, BuildConfig, BuildTarget, IrCli, LspConfig, OptLevel, PrintAsm}; use control_flow_analysis::ControlFlowGraph; pub use debug_generation::write_dwarf; use itertools::Itertools; @@ -58,9 +56,10 @@ use sway_error::warning::{CollectedTraitImpl, CompileInfo, CompileWarning, Info, use sway_features::ExperimentalFeatures; use sway_ir::{ create_o1_pass_group, register_known_passes, Context, Kind, Module, PassGroup, PassManager, - PrintPassesOpts, ARG_DEMOTION_NAME, ARG_POINTEE_MUTABILITY_TAGGER_NAME, CONST_DEMOTION_NAME, - DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_INLINE_NAME, GLOBALS_DCE_NAME, MEM2REG_NAME, - MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, SROA_NAME, + PrintPassesOpts, VerifyPassesOpts, ARG_DEMOTION_NAME, ARG_POINTEE_MUTABILITY_TAGGER_NAME, + CONST_DEMOTION_NAME, DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_INLINE_NAME, GLOBALS_DCE_NAME, + MEM2REG_NAME, MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, + SROA_NAME, }; use sway_types::span::Source; use sway_types::{SourceEngine, SourceLocation, Span}; @@ -1333,15 +1332,20 @@ pub(crate) fn compile_ast_to_ir_to_asm( // Run the passes. let print_passes_opts: PrintPassesOpts = (&build_config.print_ir).into(); - let res = - if let Err(ir_error) = pass_mgr.run_with_print(&mut ir, &pass_group, &print_passes_opts) { - Err(handler.emit_err(CompileError::InternalOwned( - ir_error.to_string(), - span::Span::dummy(), - ))) - } else { - Ok(()) - }; + let verify_passes_opts: VerifyPassesOpts = (&build_config.verify_ir).into(); + let res = if let Err(ir_error) = pass_mgr.run_with_print_verify( + &mut ir, + &pass_group, + &print_passes_opts, + &verify_passes_opts, + ) { + Err(handler.emit_err(CompileError::InternalOwned( + ir_error.to_string(), + span::Span::dummy(), + ))) + } else { + Ok(()) + }; res?; compile_ir_context_to_finalized_asm(handler, &ir, Some(build_config)) diff --git a/sway-ir/src/optimize/sroa.rs b/sway-ir/src/optimize/sroa.rs index 4cd975ed9e..ba8019e71b 100644 --- a/sway-ir/src/optimize/sroa.rs +++ b/sway-ir/src/optimize/sroa.rs @@ -250,23 +250,29 @@ pub fn sroa( indices, } in &elm_details { - let elm_index_values = indices - .iter() - .map(|&index| Value::new_u64_constant(context, index.into())) - .collect(); - let elem_ptr_ty = Type::new_typed_pointer(context, *r#type); - let elm_addr = Value::new_instruction( - context, - block, - InstOp::GetElemPtr { - base: src_val_ptr, - elem_ptr_ty, - indices: elm_index_values, - }, - ); + let elm_addr = if indices.is_empty() { + // We're looking at a pointer to a scalar, so no GEP needed. + src_val_ptr + } else { + let elm_index_values = indices + .iter() + .map(|&index| Value::new_u64_constant(context, index.into())) + .collect(); + let elem_ptr_ty = Type::new_typed_pointer(context, *r#type); + let gep = Value::new_instruction( + context, + block, + InstOp::GetElemPtr { + base: src_val_ptr, + elem_ptr_ty, + indices: elm_index_values, + }, + ); + new_insts.push(gep); + gep + }; let load = Value::new_instruction(context, block, InstOp::Load(elm_addr)); elm_local_map.insert(*offset, load); - new_insts.push(elm_addr); new_insts.push(load); } } @@ -317,20 +323,27 @@ pub fn sroa( indices, } in elm_details { - let elm_index_values = indices - .iter() - .map(|&index| Value::new_u64_constant(context, index.into())) - .collect(); - let elem_ptr_ty = Type::new_typed_pointer(context, r#type); - let elm_addr = Value::new_instruction( - context, - block, - InstOp::GetElemPtr { - base: dst_val_ptr, - elem_ptr_ty, - indices: elm_index_values, - }, - ); + let elm_addr = if indices.is_empty() { + // We're looking at a pointer to a scalar, so no GEP needed. + dst_val_ptr + } else { + let elm_index_values = indices + .iter() + .map(|&index| Value::new_u64_constant(context, index.into())) + .collect(); + let elem_ptr_ty = Type::new_typed_pointer(context, r#type); + let gep = Value::new_instruction( + context, + block, + InstOp::GetElemPtr { + base: dst_val_ptr, + elem_ptr_ty, + indices: elm_index_values, + }, + ); + new_insts.push(gep); + gep + }; let loaded_source = elm_local_map .get(&offset) .expect("memcpy source not loaded"); @@ -342,7 +355,6 @@ pub fn sroa( stored_val: *loaded_source, }, ); - new_insts.push(elm_addr); new_insts.push(store); } } diff --git a/sway-ir/src/parser.rs b/sway-ir/src/parser.rs index 59188f42fb..bfcf884144 100644 --- a/sway-ir/src/parser.rs +++ b/sway-ir/src/parser.rs @@ -21,7 +21,9 @@ pub fn parse<'eng>( }; IrError::ParseFailure(err.to_string(), found.into()) })?; - ir_builder::build_context(irmod, source_engine, experimental, backtrace)?.verify() + let ir = ir_builder::build_context(irmod, source_engine, experimental, backtrace)?; + ir.verify()?; + Ok(ir) } // ------------------------------------------------------------------------------------------------- diff --git a/sway-ir/src/pass_manager.rs b/sway-ir/src/pass_manager.rs index f2ac12da29..fda5b53bd0 100644 --- a/sway-ir/src/pass_manager.rs +++ b/sway-ir/src/pass_manager.rs @@ -169,6 +169,20 @@ pub struct PrintPassesOpts { pub passes: HashSet, } +/// Options for verifying [Pass]es in case of running them with verifying requested. +/// +/// Note that states of IR can always be verified by injecting the module verifier pass +/// and just running the passes. That approach however offers less control over the +/// verification. E.g., requiring the verification to happen only if the previous passes +/// modified the IR cannot be done by simply injecting a module verifier. +#[derive(Debug)] +pub struct VerifyPassesOpts { + pub initial: bool, + pub r#final: bool, + pub modified_only: bool, + pub passes: HashSet, +} + #[derive(Default)] pub struct PassManager { passes: FxHashMap<&'static str, Pass>, @@ -355,12 +369,13 @@ impl PassManager { } /// Run the `passes` and return true if the `passes` modify the initial `ir`. - /// The IR states are printed according to the printing options provided in `print_opts`. - pub fn run_with_print( + /// The IR states are printed and verified according to the options provided. + pub fn run_with_print_verify( &mut self, ir: &mut Context, passes: &PassGroup, print_opts: &PrintPassesOpts, + verify_opts: &VerifyPassesOpts, ) -> Result { // Empty IRs are result of compiling dependencies. We don't want to print those. fn ir_is_empty(ir: &Context) -> bool { @@ -388,6 +403,10 @@ impl PassManager { print_initial_or_final_ir(ir, "Initial"); } + if verify_opts.initial { + ir.verify()?; + } + let mut modified = false; for pass in passes.flatten_pass_group() { let modified_in_pass = self.actually_run(ir, pass)?; @@ -397,12 +416,20 @@ impl PassManager { } modified |= modified_in_pass; + if verify_opts.passes.contains(pass) && (!verify_opts.modified_only || modified_in_pass) + { + ir.verify()?; + } } if print_opts.r#final { print_initial_or_final_ir(ir, "Final"); } + if verify_opts.r#final { + ir.verify()?; + } + Ok(modified) } diff --git a/sway-ir/src/verify.rs b/sway-ir/src/verify.rs index 24269ae7bd..8f7f0ae89d 100644 --- a/sway-ir/src/verify.rs +++ b/sway-ir/src/verify.rs @@ -46,12 +46,12 @@ pub fn create_module_verifier_pass() -> Pass { impl Context<'_> { /// Verify the contents of this [`Context`] is valid. - pub fn verify(self) -> Result { + pub fn verify(&self) -> Result<(), IrError> { for (module, _) in &self.modules { let module = Module(module); self.verify_module(module)?; } - Ok(self) + Ok(()) } fn verify_module(&self, module: Module) -> Result<(), IrError> { diff --git a/sway-ir/tests/tests.rs b/sway-ir/tests/tests.rs index fa65a84de8..8240abbd07 100644 --- a/sway-ir/tests/tests.rs +++ b/sway-ir/tests/tests.rs @@ -50,7 +50,7 @@ fn run_tests bool>(sub_dir: &str, opt_fn: F) { "Pass returned false (no changes made to {}).", path.display() ); - let ir = ir.verify().unwrap_or_else(|err| { + ir.verify().unwrap_or_else(|err| { println!("{err}"); panic!(); }); diff --git a/test/src/e2e_vm_tests/harness.rs b/test/src/e2e_vm_tests/harness.rs index 77ab5c17e4..524558353d 100644 --- a/test/src/e2e_vm_tests/harness.rs +++ b/test/src/e2e_vm_tests/harness.rs @@ -286,6 +286,7 @@ pub(crate) async fn compile_to_bytes( ir: run_config.print_ir.clone(), reverse_order: false, }, + verify_ir: run_config.verify_ir.clone(), pkg: forc_pkg::PkgOpts { path: Some(root.clone()), locked: run_config.locked, diff --git a/test/src/ir_generation/mod.rs b/test/src/ir_generation/mod.rs index 3a95a80191..3dc3eca1f2 100644 --- a/test/src/ir_generation/mod.rs +++ b/test/src/ir_generation/mod.rs @@ -300,8 +300,8 @@ pub(super) async fn run( span.end(), span.as_str() ); - }) - .verify() + }); + ir.verify() .unwrap_or_else(|err| { panic!("IR verification failed for test {}:\n{err}", path.display()); }); diff --git a/test/src/main.rs b/test/src/main.rs index 0e678393aa..a84cbcdd5d 100644 --- a/test/src/main.rs +++ b/test/src/main.rs @@ -6,10 +6,10 @@ mod test_consistency; use anyhow::Result; use clap::Parser; -use forc::cli::shared::{PrintAsmCliOpt, PrintIrCliOpt}; +use forc::cli::shared::{IrCliOpt, PrintAsmCliOpt}; use forc_tracing::init_tracing_subscriber; use std::str::FromStr; -use sway_core::{BuildTarget, PrintAsm, PrintIr}; +use sway_core::{BuildTarget, IrCli, PrintAsm}; use tracing::Instrument; #[derive(Parser)] @@ -75,9 +75,13 @@ struct Cli { /// Print out the specified IR (separate options with comma), if the verbose option is on /// /// This option is ignored if tests are run in parallel. - #[arg(long, num_args(1..=18), value_parser = clap::builder::PossibleValuesParser::new(PrintIrCliOpt::cli_options()))] + #[arg(long, num_args(1..=18), value_parser = clap::builder::PossibleValuesParser::new(IrCliOpt::cli_options()))] print_ir: Option>, + /// Verify the generated Sway IR (Intermediate Representation). + #[arg(long, value_parser = clap::builder::PossibleValuesParser::new(IrCliOpt::cli_options()))] + verify_ir: Option>, + /// Print out the specified ASM (separate options with comma), if the verbose option is on /// /// This option is ignored if tests are run in parallel. @@ -188,7 +192,8 @@ pub struct RunConfig { pub verbose: bool, pub release: bool, pub update_output_files: bool, - pub print_ir: PrintIr, + pub print_ir: IrCli, + pub verify_ir: IrCli, pub print_asm: PrintAsm, pub print_bytecode: bool, pub experimental: sway_features::CliFields, @@ -231,9 +236,13 @@ async fn main() -> Result<()> { build_target, experimental: cli.experimental, update_output_files: cli.update_output_files, + verify_ir: cli + .verify_ir + .as_ref() + .map_or(IrCli::default(), |opts| IrCliOpt::from(opts).0), perf: cli.perf, // Ignore options that are not supported when running tests in parallel. - print_ir: PrintIr::none(), + print_ir: IrCli::none(), print_asm: PrintAsm::none(), print_bytecode: false, write_output: false, @@ -275,7 +284,11 @@ async fn main() -> Result<()> { print_ir: cli .print_ir .as_ref() - .map_or(PrintIr::default(), |opts| PrintIrCliOpt::from(opts).0), + .map_or(IrCli::default(), |opts| IrCliOpt::from(opts).0), + verify_ir: cli + .verify_ir + .as_ref() + .map_or(IrCli::default(), |opts| IrCliOpt::from(opts).0), print_asm: cli .print_asm .as_ref()