Skip to content

Conversation

@m-alvarez
Copy link
Contributor

As noted in #1602, bitwise operations on uint256_t are vectorized by GCC/Clang and incur store forwarding stalls when mixed with non-vectorized operations. This can be fixed by forcing the intermediate results.

@m-alvarez m-alvarez requested review from aa755 and dhil October 22, 2025 16:26
@m-alvarez m-alvarez requested a review from Baltoli as a code owner October 22, 2025 16:26
Copilot AI review requested due to automatic review settings October 22, 2025 16:26
@m-alvarez m-alvarez requested a review from andreaslyn as a code owner October 22, 2025 16:26
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses store forwarding stalls in uint256_t bitwise operations by forcing intermediate results to prevent AVX vectorization issues when mixed with non-vectorized operations.

Key Changes:

  • Wraps bitwise operation results in force() calls within the BITWISE_BINOP macro to prevent compiler vectorization that causes store forwarding stalls
Comments suppressed due to low confidence (1)

category/vm/runtime/uint256.hpp:1

  • [nitpick] The force() function is not defined or documented in the visible context. Consider adding a comment explaining what force() does (e.g., preventing compiler vectorization) to improve code readability for future maintainers.
// Copyright (C) 2025 Category Labs, Inc.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@m-alvarez m-alvarez changed the title [uint256] Force intermediate ressults of bitwise operations to avoid AVX stalls [uint256] Force intermediate results of bitwise operations to avoid AVX stalls Oct 22, 2025
Copy link
Contributor

@dhil dhil left a comment

Choose a reason for hiding this comment

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

What is the generated code diff before/after these changes?

Copy link
Contributor

@andreaslyn andreaslyn left a comment

Choose a reason for hiding this comment

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

Did you forget NOT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants