Skip to content

Skip literals that are not scalars or iotas when fusing attention - #5211

Open
ahsan-ca wants to merge 2 commits into
developfrom
fix-iota-literal
Open

Skip literals that are not scalars or iotas when fusing attention#5211
ahsan-ca wants to merge 2 commits into
developfrom
fix-iota-literal

Conversation

@ahsan-ca

Copy link
Copy Markdown
Contributor

Motivation

Technical Details

Changelog Category

Add a CHANGELOG.md entry for any option other than Not Applicable

    • Added: New functionality.
    • Changed: Changes to existing functionality.
    • Removed: Functionality or support that has been removed. (Compared to a previous release)
    • Optimized: Component performance that has been optimized or improved.
    • Resolved Issues: Known issues from a previous version that have been resolved.
    • Not Applicable: This PR is not to be included in the changelog.

Follow the LLVM AI Tool Use Policy for contributions using AI.

@ahsan-ca
ahsan-ca requested a review from pfultz2 August 28, 2026 16:33
@ahsan-ca ahsan-ca self-assigned this Aug 28, 2026
@ahsan-ca
ahsan-ca requested a review from causten as a code owner August 28, 2026 16:33
Copilot AI lite review requested due to automatic review settings August 28, 2026 16:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 refines the fuse_attention pass’s constant-capture behavior so attention fusion only pulls in “inlinable” literal constants (scalars or iota/range literals), avoiding problematic large/non-iota literals while still enabling MLIR causal-mask detection.

Changes:

  • Add a literal classifier in src/fuse_attention.cpp to restrict which literals get pulled into the attention subgraph during fusion.
  • Update existing attention-fusion tests to pass relevant literals as group inputs (instead of recreating them inside the grouped module).
  • Add a new regression test covering a packed-but-nonstandard-stride bias literal that should not be inlined as an iota/range.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/fuse_attention.cpp Adds “range literal” detection and uses it to gate which evaluable constants are pulled into attention fusion.
test/fuse_attention.cpp Updates expected fused graphs to pass literals via group inputs and adds a regression test for non-iota bias literals.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/fuse_attention.cpp
Comment on lines +118 to +127
bool is_range_literal(const literal& l)
{
bool result = false;
l.visit([&](auto x) {
result = std::adjacent_find(x.begin(), x.end(), [](auto cur, auto next) {
return not float_equal(next - cur, 1);
}) == x.end();
});
return result;
}

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comment thread src/fuse_attention.cpp
Comment on lines +122 to +124
result = std::adjacent_find(x.begin(), x.end(), [](auto cur, auto next) {
return not float_equal(next - cur, 1);
}) == x.end();
Comment thread src/fuse_attention.cpp
Comment on lines +131 to +133
if(ins->name() != "@literal")
return true;
return ins->get_shape().elements() == 1 or is_range_literal(ins->get_literal());
Comment thread test/fuse_attention.cpp Outdated
Comment on lines +361 to +362
for(std::size_t i = 0; i < bias_vec.size(); i++)
bias_vec[i] = 0.5f * i;

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/fuse_attention.cpp:126

  • next - cur can overflow for signed integer literals even after the monotonicity check (for example, {INT64_MIN, INT64_MAX}), causing undefined behavior while running the fusion pass. Compare against cur + 1 instead; next > cur guarantees that increment is safe and also expresses the iota condition directly.
        result = std::adjacent_find(x.begin(), x.end(), [](auto cur, auto next) {
                     return next <= cur or not float_equal(next - cur, 1);

Comment thread src/fuse_attention.cpp
return 0;
}

bool is_range_literal(const literal& l)
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.

2 participants