This is an archive of the discontinued LLVM Phabricator instance.

[X86] Add lowering of fminimum/fmaximum for vector operands
ClosedPublic

Authored by skatkov on May 4 2023, 4:45 AM.

Diff Detail

Event Timeline

skatkov created this revision.May 4 2023, 4:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2023, 4:45 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
skatkov requested review of this revision.May 4 2023, 4:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2023, 4:45 AM
RKSimon added inline comments.May 4 2023, 5:43 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
1395

Add these here in the existing loop

1823

Add these here in the existing loop

e-kud added a comment.May 4 2023, 7:49 PM

Maybe a few more vector tests? Existing tests cover the no-nans and no-signed-zeros case, they were added to avoid bloat of scalarization. E.g. a test with all checks (generic vectorized version) and two test with constant vectors to eliminate a zero check and a NaN check.

skatkov updated this revision to Diff 519864.May 5 2023, 8:15 AM

Handled comments.

skatkov marked 2 inline comments as done.May 5 2023, 8:16 AM

Maybe a few more vector tests? Existing tests cover the no-nans and no-signed-zeros case, they were added to avoid bloat of scalarization. E.g. a test with all checks (generic vectorized version) and two test with constant vectors to eliminate a zero check and a NaN check.

added

e-kud added inline comments.May 8 2023, 8:57 AM
llvm/test/CodeGen/X86/fminimum-fmaximum.ll
1082

Shouldn't it be a simple min(<0, 0>, x)? %x is picked anyway as a second op, regardless NaN is there or -0.0

1086

s/singned/signed

1138

Same but with max: max(<-0, -0, -0, -0>, x) should return the right answer without any checks.

skatkov updated this revision to Diff 520910.May 9 2023, 10:55 PM

I've pre-landed the new tests, fixed the name in the test and updated this revision.

Unfortunately handlining -f 0. for max and 0.f for min will not work as we want due to we check only for preferred zero.
And it is not a problem of vectorized version but scalar as well.

I propose to handle this as follow-up patch.

e-kud accepted this revision.May 10 2023, 6:13 AM

LGTM. Thanks!

This revision is now accepted and ready to land.May 10 2023, 6:13 AM
This revision was landed with ongoing or failed builds.May 10 2023, 9:42 PM
This revision was automatically updated to reflect the committed changes.