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

feat: implement -o flags for disabling optimizations #5385

Merged
merged 25 commits into from
Jan 20, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
41d7d0d
feat: implement o flags for disabling optimizations
kayagokalp Dec 12, 2023
93b0e3d
Merge branch 'master' into kayagokalp/o-flags
vaivaswatha Jan 1, 2024
2b57396
Disable some optimizations on -O0
vaivaswatha Jan 4, 2024
81dc8d6
Merge branch 'master' into kayagokalp/o-flags
vaivaswatha Jan 4, 2024
c9840d0
Provide option to the e2e test binary to specify build profile.
vaivaswatha Jan 10, 2024
3b453b5
Merge branch 'master' into kayagokalp/o-flags
vaivaswatha Jan 10, 2024
3d2a3c7
Disable referencing_local_vars_and_values test
vaivaswatha Jan 13, 2024
73646b3
Merge branch 'master' into kayagokalp/o-flags
vaivaswatha Jan 13, 2024
6ceede2
Renable `referencing_local_vars_and_values` test.
vaivaswatha Jan 16, 2024
a21e36a
Update fuel-core version in CI change
vaivaswatha Jan 16, 2024
26950ae
Merge remote-tracking branch 'origin/master' into kayagokalp/o-flags
vaivaswatha Jan 16, 2024
776d695
Fix typo during merge
vaivaswatha Jan 16, 2024
eb628d2
e2e testing: deployed contracts must use the right build profile
vaivaswatha Jan 18, 2024
54c6929
Merge branch 'master' into kayagokalp/o-flags
vaivaswatha Jan 18, 2024
6b1b48f
Testing: build sway-lib-std in release mode
vaivaswatha Jan 18, 2024
52d0db1
Run release version of forc for sway-lib-std test
vaivaswatha Jan 18, 2024
4896aaf
Update sdk-harness abi path to release
vaivaswatha Jan 18, 2024
c5fe9ef
Update sdk-harness abi path to release missed out in previous commit
vaivaswatha Jan 18, 2024
9e0fda9
attend to review comments
vaivaswatha Jan 19, 2024
89cc106
Merge branch 'master' into kayagokalp/o-flags
sdankel Jan 19, 2024
0537073
Merge branch 'master' into kayagokalp/o-flags
sdankel Jan 19, 2024
ca332a2
Merge branch 'master' into kayagokalp/o-flags
vaivaswatha Jan 19, 2024
9bc886d
Merge branch 'kayagokalp/o-flags' of github.com:FuelLabs/sway into ka…
vaivaswatha Jan 19, 2024
df2be76
Fix missing requires in CI
vaivaswatha Jan 19, 2024
a1de5bc
Merge branch 'master' into kayagokalp/o-flags
sdankel Jan 20, 2024
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
19 changes: 18 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,23 @@ jobs:
- name: Cargo Run E2E Tests (Fuel VM)
run: cargo run --locked --release --bin test -- --locked

cargo-run-e2e-test-release:
runs-on: ubuntu-latest
services:
fuel-core:
image: ghcr.io/fuellabs/fuel-core:v0.22.0
tritao marked this conversation as resolved.
Show resolved Hide resolved
ports:
- 4000:4000
steps:
- uses: actions/checkout@v3
- name: Install toolchain
uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ env.RUST_VERSION }}
- uses: Swatinem/rust-cache@v2
- name: Cargo Run E2E Tests (Fuel VM)
run: cargo run --locked --release --bin test -- --locked --release

cargo-run-e2e-test-evm:
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -328,7 +345,7 @@ jobs:
toolchain: ${{ env.RUST_VERSION }}
- uses: Swatinem/rust-cache@v2
- name: Build All Tests
run: cargo run --locked -p forc -- build --locked --path ./test/src/sdk-harness
run: cargo run --locked --release -p forc -- build --release --locked --path ./test/src/sdk-harness
- name: Cargo Test sway-lib-std
run: cargo test --locked --release --manifest-path ./test/src/sdk-harness/Cargo.toml -- --nocapture

Expand Down
3 changes: 3 additions & 0 deletions forc-pkg/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ pub struct BuildProfile {
#[serde(default)]
pub error_on_warnings: bool,
pub reverse_results: bool,
pub optimization_level: usize,
tritao marked this conversation as resolved.
Show resolved Hide resolved
#[serde(default)]
pub experimental: ExperimentalFlags,
}
Expand Down Expand Up @@ -726,6 +727,7 @@ impl BuildProfile {
json_abi_with_callpaths: false,
error_on_warnings: false,
reverse_results: false,
optimization_level: 0,
experimental: ExperimentalFlags {
new_encoding: false,
},
Expand All @@ -747,6 +749,7 @@ impl BuildProfile {
json_abi_with_callpaths: false,
error_on_warnings: false,
reverse_results: false,
optimization_level: 1,
experimental: ExperimentalFlags {
new_encoding: false,
},
Expand Down
1 change: 1 addition & 0 deletions forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1564,6 +1564,7 @@ pub fn sway_build_config(
.with_include_tests(build_profile.include_tests)
.with_time_phases(build_profile.time_phases)
.with_metrics(build_profile.metrics_outfile.clone())
.with_optimization_level(build_profile.optimization_level)
.with_experimental(sway_core::ExperimentalFlags {
new_encoding: build_profile.experimental.new_encoding,
});
Expand Down
9 changes: 9 additions & 0 deletions sway-core/src/build_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ pub struct BuildConfig {
pub(crate) print_finalized_asm: bool,
pub(crate) print_ir: bool,
pub(crate) include_tests: bool,
pub(crate) optimization_level: usize,
pub time_phases: bool,
pub metrics_outfile: Option<String>,
pub experimental: ExperimentalFlags,
Expand Down Expand Up @@ -93,6 +94,7 @@ impl BuildConfig {
include_tests: false,
time_phases: false,
metrics_outfile: None,
optimization_level: 0,
experimental: ExperimentalFlags::default(),
}
}
Expand Down Expand Up @@ -146,6 +148,13 @@ impl BuildConfig {
}
}

pub fn with_optimization_level(self, optimization_level: usize) -> Self {
Self {
optimization_level,
..self
}
}

/// Whether or not to include test functions in parsing, type-checking and codegen.
///
/// This should be set to `true` by invocations like `forc test` or `forc check --tests`.
Expand Down
25 changes: 18 additions & 7 deletions sway-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ use std::sync::Arc;
use sway_ast::AttributeDecl;
use sway_error::handler::{ErrorEmitted, Handler};
use sway_ir::{
create_o1_pass_group, register_known_passes, Context, Kind, Module, PassManager,
ARGDEMOTION_NAME, CONSTDEMOTION_NAME, DCE_NAME, MEM2REG_NAME, MEMCPYOPT_NAME,
MISCDEMOTION_NAME, MODULEPRINTER_NAME, RETDEMOTION_NAME, SIMPLIFYCFG_NAME, SROA_NAME,
create_o1_pass_group, register_known_passes, Context, Kind, Module, PassGroup, PassManager,
ARGDEMOTION_NAME, CONSTDEMOTION_NAME, DCE_NAME, INLINE_MODULE_NAME, MEM2REG_NAME,
MEMCPYOPT_NAME, MISCDEMOTION_NAME, MODULEPRINTER_NAME, RETDEMOTION_NAME, SIMPLIFYCFG_NAME,
SROA_NAME,
};
use sway_types::constants::DOC_COMMENT_ATTRIBUTE_NAME;
use sway_types::SourceEngine;
Expand Down Expand Up @@ -804,7 +805,14 @@ pub(crate) fn compile_ast_to_ir_to_asm(
// Initialize the pass manager and register known passes.
let mut pass_mgr = PassManager::default();
register_known_passes(&mut pass_mgr);
let mut pass_group = create_o1_pass_group();
let mut pass_group = PassGroup::default();

if build_config.optimization_level != 0 {
pass_group.append_group(create_o1_pass_group());
} else {
// Inlining is necessary until #4899 is resolved.
pass_group.append_pass(INLINE_MODULE_NAME);
}

// Target specific transforms should be moved into something more configured.
if build_config.build_target == BuildTarget::Fuel {
Expand All @@ -823,9 +831,12 @@ pub(crate) fn compile_ast_to_ir_to_asm(
// Run a DCE and simplify-cfg to clean up any obsolete instructions.
pass_group.append_pass(DCE_NAME);
pass_group.append_pass(SIMPLIFYCFG_NAME);
pass_group.append_pass(SROA_NAME);
pass_group.append_pass(MEM2REG_NAME);
pass_group.append_pass(DCE_NAME);

if build_config.optimization_level != 0 {
pass_group.append_pass(SROA_NAME);
pass_group.append_pass(MEM2REG_NAME);
pass_group.append_pass(DCE_NAME);
}
}

if build_config.print_ir {
Expand Down
6 changes: 6 additions & 0 deletions test/src/e2e_vm_tests/harness.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use anyhow::{anyhow, bail, Result};
use colored::Colorize;
use forc::cli::shared::BuildProfile;
use forc_client::{
cmd::{Deploy as DeployCommand, Run as RunCommand},
op::{deploy, run},
Expand Down Expand Up @@ -72,6 +73,10 @@ pub(crate) async fn deploy_contract(file_name: &str, run_config: &RunConfig) ->
},
signing_key: Some(SecretKey::from_str(SECRET_KEY).unwrap()),
default_salt: true,
build_profile: BuildProfile {
release: run_config.release,
..Default::default()
},
..Default::default()
})
.await
Expand Down Expand Up @@ -252,6 +257,7 @@ pub(crate) async fn compile_to_bytes(file_name: &str, run_config: &RunConfig) ->
let manifest_dir = env!("CARGO_MANIFEST_DIR");
let build_opts = forc_pkg::BuildOpts {
build_target: run_config.build_target,
release: run_config.release,
pkg: forc_pkg::PkgOpts {
path: Some(format!(
"{manifest_dir}/src/e2e_vm_tests/test_programs/{file_name}",
Expand Down
29 changes: 29 additions & 0 deletions test/src/e2e_vm_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use anyhow::{anyhow, bail, Result};
use assert_matches::assert_matches;
use colored::*;
use core::fmt;
use forc_pkg::BuildProfile;
use fuel_vm::fuel_tx;
use fuel_vm::prelude::*;
use regex::Regex;
Expand Down Expand Up @@ -68,6 +69,7 @@ struct TestDescription {
validate_abi: bool,
validate_storage_slots: bool,
supported_targets: HashSet<BuildTarget>,
unsupported_profiles: Vec<&'static str>,
checker: filecheck::Checker,
}

Expand Down Expand Up @@ -539,6 +541,12 @@ pub async fn run(filter_config: &FilterConfig, run_config: &RunConfig) -> Result
if filter_config.first_only && !tests.is_empty() {
tests = vec![tests.remove(0)];
}
let cur_profile = if run_config.release {
BuildProfile::RELEASE
} else {
BuildProfile::DEBUG
};
tests.retain(|t| !t.unsupported_profiles.contains(&cur_profile));

// Run tests
let context = TestContext {
Expand Down Expand Up @@ -866,6 +874,15 @@ fn parse_test_toml(path: &Path) -> Result<TestDescription> {
.map(get_test_abi_from_value)
.collect::<Result<Vec<BuildTarget>>>()?;

// Check for not supported build profiles. Default is empty.
let unsupported_profiles = toml_content
.get("unsupported_profiles")
.map(|v| v.as_array().cloned().unwrap_or_default())
.unwrap_or_default()
.iter()
.map(get_build_profile_from_value)
.collect::<Result<Vec<&'static str>>>()?;

let supported_targets = HashSet::from_iter(if supported_targets.is_empty() {
vec![BuildTarget::Fuel]
} else {
Expand All @@ -883,6 +900,7 @@ fn parse_test_toml(path: &Path) -> Result<TestDescription> {
validate_abi,
validate_storage_slots,
supported_targets,
unsupported_profiles,
checker,
})
}
Expand All @@ -897,6 +915,17 @@ fn get_test_abi_from_value(value: &toml::Value) -> Result<BuildTarget> {
}
}

fn get_build_profile_from_value(value: &toml::Value) -> Result<&'static str> {
match value.as_str() {
Some(profile) => match profile {
BuildProfile::DEBUG => Ok(BuildProfile::DEBUG),
BuildProfile::RELEASE => Ok(BuildProfile::RELEASE),
_ => Err(anyhow!(format!("Unknown build profile"))),
},
None => Err(anyhow!("Invalid TOML value")),
}
}

fn get_expected_result(toml_content: &toml::Value) -> Result<TestResult> {
fn get_action_value(action: &toml::Value, expected_value: &toml::Value) -> Result<TestResult> {
match (action.as_str(), expected_value) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
category = "run"
category = "disabled"
expected_result = { action = "revert", value = 0 }
tritao marked this conversation as resolved.
Show resolved Hide resolved
validate_abi = false
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
category = "run"
category = "disabled"
expected_result = { action = "revert", value = 0 }
validate_abi = false
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ category = "run"
expected_result = { action = "return", value = 0 }
validate_abi = true
expected_warnings = 2
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ fn reference_local_reference_var_and_value_not_inlined<T>()
}

#[inline(always)]
fn reference_zero_sized_local_var_and_value<T>(is_inlined: bool)
fn reference_zero_sized_local_var_and_value<T>()
where T: Eq + New + ZeroSize
{
assert(__size_of::<T>() == 0);
Expand All @@ -127,21 +127,6 @@ fn reference_zero_sized_local_var_and_value<T>(is_inlined: bool)
let r_val_ptr = asm(r: r_val) { r: raw_ptr };
let r_dummy_ptr = asm(r: r_dummy) { r: raw_ptr };

// If there is no inlining and mixing with other test functions,
// since the size of `T` is zero, means allocates zero memory,
// both created values will be on the same memory location.
// The dummy value will also be on the same location.
// In case of inlining with other test functions we can get
// the two variables position separately from each other, intermixed
// with the locals coming from other functions.
// Note that we rely on the optimization which will remove the local
// variables for the references. This assumption is fine,
// the test should never be flaky.
if (!is_inlined) {
assert(r_x_1_ptr == r_val_ptr);
assert(r_x_1_ptr == r_dummy_ptr);
}

assert(r_x_1_ptr == r_x_2_ptr);

let r_x_1_ptr_val = r_x_1_ptr.read::<T>();
Expand All @@ -164,7 +149,7 @@ fn reference_zero_sized_local_var_and_value<T>(is_inlined: bool)
fn reference_zero_sized_local_var_and_value_not_inlined<T>()
where T: Eq + New + ZeroSize
{
reference_zero_sized_local_var_and_value::<T>(false)
reference_zero_sized_local_var_and_value::<T>()
}

#[inline(never)]
Expand All @@ -186,8 +171,8 @@ fn test_all_inlined() {
reference_local_var_and_value::<raw_ptr>();
reference_local_var_and_value::<raw_slice>();

reference_zero_sized_local_var_and_value::<EmptyStruct>(true);
reference_zero_sized_local_var_and_value::<[u64;0]>(true);
reference_zero_sized_local_var_and_value::<EmptyStruct>();
reference_zero_sized_local_var_and_value::<[u64;0]>();

reference_local_reference_var_and_value::<()>();
reference_local_reference_var_and_value::<bool>();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
category = "run"
expected_result = { action = "return", value = 42 }
validate_abi = true
expected_warnings = 9 # TODO-IG: Should be zero. DCA problem? Investigate why there are "This method is never called." errors for some of the types.
expected_warnings = 90 # TODO-IG: Should be zero. DCA problem? Investigate why there are "This method is never called." errors for some of the types.
tritao marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run"
expected_result = { action = "return_data", value = "0000000000000000000000000000000000000000000000000000000000000001" }
validate_abi = true
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run_on_node"
expected_result = { action = "result", value = 1 }
contracts = ["should_pass/test_contracts/array_of_structs_contract"]
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run_on_node"
expected_result = { action = "result", value = 1 }
contracts = ["should_pass/test_contracts/balance_test_contract", "should_pass/test_contracts/test_fuel_coin_contract"]
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run_on_node"
expected_result = { action = "result", value = 1 }
contracts = ["should_pass/test_contracts/balance_test_contract"]
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run_on_node"
expected_result = { action = "result", value = 1 }
contracts = ["should_pass/test_contracts/abi_with_tuples_contract"]
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run_on_node"
expected_result = { action = "result", value = 4242 }
contracts = ["should_pass/test_contracts/basic_storage"]
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run_on_node"
expected_result = { action = "result", value = 0 }
contracts = ["should_pass/test_contracts/contract_with_type_aliases"]
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run_on_node"
expected_result = { action = "result", value = 1 }
contracts = ["should_pass/test_contracts/increment_contract"]
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run_on_node"
expected_result = { action = "result", value = 171 }
contracts = ["should_pass/test_contracts/storage_enum_contract"]
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run_on_node"
expected_result = { action = "result", value = 1 }
contracts = ["should_pass/test_contracts/auth_testing_contract"]
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run_on_node"
expected_result = { action = "result", value = 1 }
contracts = ["should_pass/test_contracts/context_testing_contract"]
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run_on_node"
expected_result = { action = "result", value = 1 }
contracts = ["should_pass/test_contracts/nested_struct_args_contract"]
unsupported_profiles = ["debug"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
category = "run_on_node"
expected_result = { action = "result", value = 1 }
contracts = ["should_pass/test_contracts/storage_access_contract"]
unsupported_profiles = ["debug"]
Loading
Loading