Skip to content

<complex>: use hardware FMA on Clang too - #5721

Merged
Stephan T. Lavavej (StephanTLavavej) merged 3 commits into
microsoft:mainfrom
AlexGuteniev:its-there-if-you-ask
Sep 22, 2025
Merged

<complex>: use hardware FMA on Clang too#5721
Stephan T. Lavavej (StephanTLavavej) merged 3 commits into
microsoft:mainfrom
AlexGuteniev:its-there-if-you-ask

Conversation

@AlexGuteniev

Copy link
Copy Markdown
Contributor

Yes it does not compile without [[gnu::target("fma")]] magic spell.

Comment thread stl/inc/complex Outdated
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for figuring out the magic incantation here! 😻

I observe that this is inconsistent with the spelling of

#pragma clang attribute _STD_ATOMIC_HEADER.push([[gnu::target("cx16")]], apply_to = function)
but I don't care enough to iterate.

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) moved this from Initial Review to Ready To Merge in STL Code Reviews Sep 17, 2025
@AlexGuteniev

Copy link
Copy Markdown
Contributor Author

I observe that this is inconsistent with the spelling of

If you want consistency, I'd change <atomic> actually.

The spell in <atomic> allows cx16 insn in any function, and this theoretically permits generating one out of thin air. In contrast, the approach in this PR allows fma only in specific function, for which we have runtime check.

<atomic> does not centralize cx16, and there would be few functions, so it takes more changes, but still may be worth doing.

@StephanTLavavej

Copy link
Copy Markdown
Member

Oh, all I meant was the spelling of __gnu__::__target__.

@AlexGuteniev

Copy link
Copy Markdown
Contributor Author

Aha! It was suggested by Nikolas Klauser (@philnik777). Makes sense as a defense from user's macro, #define gnu 1 is allowed. And in pragma there's no preprocessor, since pragma itself is considered preprocessor.

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) moved this from Ready To Merge to Merging in STL Code Reviews Sep 19, 2025
@StephanTLavavej

Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 2510fe9 into microsoft:main Sep 22, 2025
39 checks passed
@github-project-automation github-project-automation Bot moved this from Merging to Done in STL Code Reviews Sep 22, 2025
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for multiplying and adding to the compilers where we use these intrinsics! 😹 ✖️ ➕

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

Labels

performance Must go faster

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants