Skip to content

Conversation

@mcourteaux
Copy link
Contributor

No description provided.

@abadams
Copy link
Member

abadams commented Oct 23, 2025

Uh oh, looks like this doesn't do anything anymore. We need to figure out the correct way to enable the equivalent of -fp-contract=fast

@mcourteaux
Copy link
Contributor Author

Feel free to take the lead on this, and close this PR if you have better changes in mind. I made this PR just in case the fix is this simple, and all the build bots go green. I'm not really in the mood for LLVM shenanigans. 🥴

@abadams
Copy link
Member

abadams commented Oct 23, 2025

Maybe it's this?

    options.AllowFPOpFusion = llvm::FPOpFusion::Fast;

@abadams
Copy link
Member

abadams commented Oct 23, 2025

Oh we already set AllowFPOpFusion. And reassociation is turned on in set_fast_fp_math. I think this PR is the correct change.

@mcourteaux
Copy link
Contributor Author

Maybe it's this?

    options.AllowFPOpFusion = llvm::FPOpFusion::Fast;

I see: the build bots could go green, and we still lose the performance benefits of FP optimizations. I can add that line for LLVM 21+ if you think that's it.

@mcourteaux
Copy link
Contributor Author

Oh we already set AllowFPOpFusion. And reassociation is turned on in set_fast_fp_math. I think this PR is the correct change.

Okay, then we'll wait for the bots. 😄 Thanks for chiming in so quickly.

@mcourteaux
Copy link
Contributor Author

Any idea what's going on with correctness/extern_sort on Windows?

@mcourteaux
Copy link
Contributor Author

It's a fluke I think. It did work on my other PR: https://buildbot.halide-lang.org/master/#/builders/267/builds/187#bb-step-39 which is rebased on this PR.

@alexreinking
Copy link
Member

It's a fluke I think. It did work on my other PR: https://buildbot.halide-lang.org/master/#/builders/267/builds/187#bb-step-39 which is rebased on this PR.

I just triggered a rerun of that workflow. If it fails again, we can look more closely at it.

@mcourteaux
Copy link
Contributor Author

Yeiiiy Thanks a lot! All green! Only need formal approval from someone.

@alexreinking alexreinking merged commit cbd541d into halide:main Oct 24, 2025
19 checks passed
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.

3 participants