This is an archive of the discontinued LLVM Phabricator instance.

[X86] Add back fmaddsub intrinsics to work towards fixing the strict fp implementation
ClosedPublic

Authored by craig.topper on Feb 7 2020, 4:53 PM.

Details

Summary

Previously we emitted an fmadd and a fmadd+fneg and combined them with a shufflevector. But this doesn't follow the correct exception behavior for unselected elements so the backend can't merge them into the fmaddsub/fmsubadd instructions.

This patch restores the the fmaddsub intrinsics so we don't have two arithmetic operations. We lose out on optimization opportunity in the non-strict FP case, but I don't think this is a big loss. If someone gives us a test case we can look into adding instcombine/dagcombine improvements. I'd rather not have the frontend do completely different things for strict and non-strict.

This still has problems because target specific intrinsics don't support strict semantics yet. We also still have all of the problems with masking. But we at least generate the right instruction in constrained mode now.

Diff Detail

Event Timeline

craig.topper created this revision.Feb 7 2020, 4:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2020, 4:53 PM

I agree that its unlikely we won't lose much combine potential - we could add demanded elts support to simplify to fmadd/fmsub but that isn't that likely tbh.

Can the the fmsubadd variants still safely use the fneg ?

Are we missing AVX512 FMADDSUB_RND variants?

I agree that its unlikely we won't lose much combine potential - we could add demanded elts support to simplify to fmadd/fmsub but that isn't that likely tbh.

Can the the fmsubadd variants still safely use the fneg ?

Are we missing AVX512 FMADDSUB_RND variants?

I think using neg on the input to represent fmsubadd should be fine. Fneg doesn’t cause exceptions and it shouldn’t affect the rounding behavior if it’s on the input.

We still have rounding intrinsics for avx512 those were never removed.

This revision is now accepted and ready to land.Feb 24 2020, 10:30 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 24 2020, 12:11 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript