Skip to content

Revert "Update stable FA3 API (flash_api_stable.cpp) to be inline with the unstable ABI port (flash_api.cpp)" - #160

Merged
MatthewBonanni merged 1 commit into
mainfrom
revert-152-update_build_to_torch_stable_abi
Jul 10, 2026
Merged

Revert "Update stable FA3 API (flash_api_stable.cpp) to be inline with the unstable ABI port (flash_api.cpp)"#160
MatthewBonanni merged 1 commit into
mainfrom
revert-152-update_build_to_torch_stable_abi

Conversation

@LucasWilkinson

@LucasWilkinson LucasWilkinson commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Reverts #152

This broke FLASH_ATTN_MLA_SPARSE (vllm-project/vllm#48221) not to mention changes a bunch of heuristics

@LucasWilkinson
LucasWilkinson marked this pull request as ready for review July 10, 2026 17:18

@MatthewBonanni MatthewBonanni left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@MatthewBonanni
MatthewBonanni merged commit bb9a72e into main Jul 10, 2026
3 of 4 checks passed
MatthewBonanni added a commit to MatthewBonanni/flash-attention that referenced this pull request Jul 10, 2026
Incorporates vllm-project#160 (revert of vllm-project#152, the torch stable-ABI port that broke
FLASH_ATTN_MLA_SPARSE). Net change: hopper stable-ABI files only; no
cute-DSL / FP8 changes.
@MatthewBonanni MatthewBonanni mentioned this pull request Jul 10, 2026
@Harry-Chen

Copy link
Copy Markdown
Member

not to mention changes a bunch of heuristics

@cleonard530 Just to check, did you notice this when drafting your PR? I personally glanced over the changes and did not notice this kind of changes

@cleonard530

Copy link
Copy Markdown

not to mention changes a bunch of heuristics

@cleonard530 Just to check, did you notice this when drafting your PR? I personally glanced over the changes and did not notice this kind of changes

I just checked this again. There are definitely a lot of heuristic changes between versions of flash_api_stable.cpp, but they were intentional to align flash_api_stable.cpp with flash_api.cpp (the diff between flash_api.cpp and flash_api_stable.cpp can be viewed here 7da1d8c).

@LucasWilkinson, are these the changes you're talking about between versions of flash_api_stable.cpp? If so, how should we proceed with migrating to the stable ABI? We can align flash_api_stable.cpp with flash_api.cpp first, before we change the build, and do more checks and testing that may not be covered by the CI before updating the CMake to use flash_api_stable.

@janeyx99

Copy link
Copy Markdown

Oh no this is bad :( But I'm a bit confused, as a pretty important key of the migration is that we shouldn't change functionality. @cleonard530 could you point out where the heuristics are different? From your diff and what you linked it looks like flash_api_stable and flash_api should be aligned with each other? Where do we deviate from the existing vllm behavior?

@janeyx99

Copy link
Copy Markdown

@LucasWilkinson I don't think this revert is warranted--I put some context in vllm-project/vllm#48221 (comment), can you confirm and let me know if there's anything I missed?

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.

5 participants