From 3dc84349d94cfbc8349dde72f8170961b8a236e7 Mon Sep 17 00:00:00 2001 From: LJ <81748770+elle-j@users.noreply.github.com> Date: Thu, 9 Apr 2026 17:33:09 +0200 Subject: [PATCH] Apply LLVM patches for deterministic operand order and correct peephole optimization for `minsize` (#500) ## Description Applies LLVM patches that have not been backported to the major version used in our submodule: * [LLVM PR #163679](https://github.com/llvm/llvm-project/pull/163679) * *"[RISCV] Allow large div peephole optimization for minsize"* * Fixes peephole optimization of large integers when `minsize` function attribute is set. * (Our revive codebase used a workaround that can be removed once this 500 PR is merged and we publish a new LLVM release.) * [LLVM PR #190219](https://github.com/llvm/llvm-project/pull/190219) * *"[TargetLowering] Speculative fix for a non-determinism issue between different compilers"* * Operand ordering of `mul i256` were different depending on the target ABI when building LLVM, producing different RISC-V and PVM bytecode for our Windows vs macOS/linux builds. * The bytecode divergence (yet functionally equivalent) affected Solidity contracts containing a value [multiplied by itself](https://github.com/llvm/llvm-project/issues/190148) (e.g. via explicit squaring or exponentiation), manifesting primarily when using optimization level `-O0`. --- .github/actions/get-llvm/action.yml | 9 +++-- CHANGELOG.md | 1 + crates/llvm-builder/src/lib.rs | 2 + crates/llvm-builder/src/llvm_path.rs | 3 ++ crates/llvm-builder/src/utils.rs | 59 ++++++++++++++++++++++++---- crates/llvm-builder/tests/common.rs | 2 +- patches/.gitattributes | 1 + patches/llvm/llvm-pr-163679.patch | 15 +++++++ patches/llvm/llvm-pr-190219.patch | 18 +++++++++ 9 files changed, 99 insertions(+), 11 deletions(-) create mode 100644 patches/.gitattributes create mode 100644 patches/llvm/llvm-pr-163679.patch create mode 100644 patches/llvm/llvm-pr-190219.patch diff --git a/.github/actions/get-llvm/action.yml b/.github/actions/get-llvm/action.yml index f2b0cdd..71604fa 100644 --- a/.github/actions/get-llvm/action.yml +++ b/.github/actions/get-llvm/action.yml @@ -8,7 +8,7 @@ inputs: target: required: true version: - description: "LLVM release version (e.g., 'llvm-18.1.8'). If not specified, uses the latest LLVM release." + description: "LLVM release version (e.g., 'llvm-18.1.8'). If not specified, uses the latest published LLVM release. To test a specific draft release, use the full name (e.g. 'llvm-21.1.8-revive.0f80912')." required: false default: "" @@ -114,7 +114,9 @@ runs: core.setFailed(`Artifact for '${target}' not found in release ${llvmRelease.tag_name} (${llvmRelease.html_url})`); process.exit(); } - return asset.browser_download_url; + // Use the API URL (rather than `browser_download_url`) so that + // the authorization header is respected for draft releases. + return asset.url; } if (versionPrefix) { @@ -127,7 +129,8 @@ runs: - name: download shell: bash run: | - curl -sSLo llvm.tar.gz ${{ steps.find.outputs.result }} + # Pass the authorization token to allow downloading draft release assets. + curl -sSLo llvm.tar.gz -H "Authorization: Bearer ${{ github.token }}" -H "Accept: application/octet-stream" ${{ steps.find.outputs.result }} - name: unpack shell: bash diff --git a/CHANGELOG.md b/CHANGELOG.md index 780d25e..d39af0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ This is a development pre-release. - npm package: The `--bin` CLI flag was not producing bytecode because the `outputSelection` was not updated to explicitly requested it. - Yul mode now properly exits when solc reports validation errors (e.g., invalid hex literals), matching `--standard-json` behavior. [#477](https://github.com/paritytech/revive/issues/477) +- Fixed a bytecode divergence (yet functionally equivalent) between the resolc Linux/macOS builds and the Windows build for Solidity contracts containing a value multiplied by itself (e.g. via explicit squaring or exponentiation), manifesting primarily when using optimization level `-O0`. [#500](https://github.com/paritytech/revive/pull/500) ### Changed diff --git a/crates/llvm-builder/src/lib.rs b/crates/llvm-builder/src/lib.rs index fd22a2b..f1b9bd9 100644 --- a/crates/llvm-builder/src/lib.rs +++ b/crates/llvm-builder/src/lib.rs @@ -82,6 +82,8 @@ pub fn build( ) } + utils::apply_patches()?; + std::fs::create_dir_all(llvm_path::DIRECTORY_LLVM_TARGET.get().unwrap())?; if cfg!(target_arch = "x86_64") { diff --git a/crates/llvm-builder/src/llvm_path.rs b/crates/llvm-builder/src/llvm_path.rs index 4b70d09..d7abbab 100644 --- a/crates/llvm-builder/src/llvm_path.rs +++ b/crates/llvm-builder/src/llvm_path.rs @@ -20,6 +20,9 @@ impl LLVMPath { /// The Emscripten SDK source directory. pub const DIRECTORY_EMSDK_SOURCE: &'static str = "./emsdk/"; + /// The directory containing patches to apply to the LLVM source. + pub const DIRECTORY_PATCHES: &'static str = "./patches/llvm/"; + /// Returns the path to the `llvm` stage 1 host LLVM source module directory. pub fn llvm_host_module_llvm() -> anyhow::Result { let mut path = PathBuf::from(Self::DIRECTORY_LLVM_HOST_SOURCE); diff --git a/crates/llvm-builder/src/utils.rs b/crates/llvm-builder/src/utils.rs index f9b9e5a..3712026 100644 --- a/crates/llvm-builder/src/utils.rs +++ b/crates/llvm-builder/src/utils.rs @@ -1,6 +1,6 @@ //! The LLVM builder utilities. -use std::fs::File; +use std::fs::{read_dir, File}; use std::path::Path; use std::path::PathBuf; use std::process::Command; @@ -10,12 +10,6 @@ use std::time::Duration; use anyhow::Context; use path_slash::PathBufExt; -/// The LLVM host repository URL. -pub const LLVM_HOST_SOURCE_URL: &str = "https://github.com/llvm/llvm-project"; - -/// The LLVM host repository tag. -pub const LLVM_HOST_SOURCE_TAG: &str = "llvmorg-18.1.8"; - /// The minimum required XCode version. pub const XCODE_MIN_VERSION: u32 = 11; @@ -220,6 +214,57 @@ pub fn install_emsdk() -> anyhow::Result<()> { Ok(()) } +/// Applies all LLVM `.patch` files to the LLVM source. +/// +/// Patches that have already been applied are skipped. +pub fn apply_patches() -> anyhow::Result<()> { + let patches_directory = PathBuf::from(crate::LLVMPath::DIRECTORY_PATCHES); + if !patches_directory.exists() { + log::debug!("no patches directory found, skipping"); + return Ok(()); + } + + let llvm_source = PathBuf::from(crate::LLVMPath::DIRECTORY_LLVM_SOURCE); + let mut patches: Vec<_> = read_dir(&patches_directory)? + .filter_map(|entry| entry.ok()) + .map(|entry| entry.path()) + .filter(|path| { + path.extension() + .is_some_and(|extension| extension == "patch") + }) + .collect(); + patches.sort(); + + for patch in patches { + let patch = absolute_path(&patch)?; + let already_applied = Command::new("git") + .args(["apply", "--reverse", "--check"]) + .arg(&patch) + .current_dir(&llvm_source) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .map(|status| status.success()) + .unwrap_or(false); + + if already_applied { + log::info!("patch already applied, skipping: {}", patch.display()); + continue; + } + + log::info!("applying patch: {}", patch.display()); + command( + Command::new("git") + .args(["apply"]) + .arg(&patch) + .current_dir(&llvm_source), + &format!("applying patch {}", patch.display()), + )?; + } + + Ok(()) +} + /// The LLVM target directory default path. pub fn directory_target_llvm(target_env: crate::target_env::TargetEnv) -> PathBuf { crate::llvm_path::DIRECTORY_LLVM_TARGET diff --git a/crates/llvm-builder/tests/common.rs b/crates/llvm-builder/tests/common.rs index 22793b0..b639009 100644 --- a/crates/llvm-builder/tests/common.rs +++ b/crates/llvm-builder/tests/common.rs @@ -24,7 +24,7 @@ impl TestDir { "submodule", "add", "-b", - "release/18.x", + "release/21.x", "https://github.com/llvm/llvm-project.git", "llvm", ]) diff --git a/patches/.gitattributes b/patches/.gitattributes new file mode 100644 index 0000000..dbf3d25 --- /dev/null +++ b/patches/.gitattributes @@ -0,0 +1 @@ +*.patch eol=lf diff --git a/patches/llvm/llvm-pr-163679.patch b/patches/llvm/llvm-pr-163679.patch new file mode 100644 index 0000000..d4cacfb --- /dev/null +++ b/patches/llvm/llvm-pr-163679.patch @@ -0,0 +1,15 @@ +diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp +index eb875583ff..169465e18f 100644 +--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp ++++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp +@@ -24830,7 +24830,8 @@ bool RISCVTargetLowering::isIntDivCheap(EVT VT, AttributeList Attr) const { + // instruction, as it is usually smaller than the alternative sequence. + // TODO: Add vector division? + bool OptSize = Attr.hasFnAttr(Attribute::MinSize); +- return OptSize && !VT.isVector(); ++ return OptSize && !VT.isVector() && ++ VT.getSizeInBits() <= getMaxDivRemBitWidthSupported(); + } + + bool RISCVTargetLowering::preferScalarizeSplat(SDNode *N) const { + diff --git a/patches/llvm/llvm-pr-190219.patch b/patches/llvm/llvm-pr-190219.patch new file mode 100644 index 0000000..40ca013 --- /dev/null +++ b/patches/llvm/llvm-pr-190219.patch @@ -0,0 +1,18 @@ +diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp +index e0597988e..56520d911 100644 +--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp ++++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp +@@ -11055,10 +11055,10 @@ void TargetLowering::forceExpandMultiply(SelectionDAG &DAG, const SDLoc &dl, + // If HiLHS and HiRHS are set, multiply them by the opposite low part and add + // the products to Hi. + if (HiLHS) { ++ SDValue RHLL = DAG.getNode(ISD::MUL, dl, VT, HiRHS, LHS); ++ SDValue RLLH = DAG.getNode(ISD::MUL, dl, VT, RHS, HiLHS); + Hi = DAG.getNode(ISD::ADD, dl, VT, Hi, +- DAG.getNode(ISD::ADD, dl, VT, +- DAG.getNode(ISD::MUL, dl, VT, HiRHS, LHS), +- DAG.getNode(ISD::MUL, dl, VT, RHS, HiLHS))); ++ DAG.getNode(ISD::ADD, dl, VT, RHLL, RLLH)); + } + } +