This is an archive of the discontinued LLVM Phabricator instance.

[X86][SSE2] Follow up patch for D87236 to avoid the regression regarding horizontal unsigned 16 bit minimums and maximums.
Changes PlannedPublic

Authored by RKSimon on Sep 21 2020, 8:34 AM.

Details

Summary

This is a follow up patch for D87236 based on the comments about the test regressions.

The new code is even slightly better than it was originally because the first sign bit flip is pulled before the first shuffle. Maybe this should also be done for lowering for 8 bit signed minimums and maximums.

An entirely alternative approach to fix the regression might be to switch to the "sign bit flip" method depending on some heuristic based on the number of chained UMIN/UMAX instructions. For example when calculating the median of 3 integers via max(min(x, y), min(max(x, y), z)), we also want to use the old way, I think. However this is more complex to implement.

Diff Detail

Event Timeline

TomHender created this revision.Sep 21 2020, 8:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2020, 8:34 AM
TomHender requested review of this revision.Sep 21 2020, 8:34 AM
RKSimon added inline comments.Sep 29 2020, 10:07 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
39249

Is it worth pulling this in as well?

39282

clang-format this

We might be better off waiting until D88787 lands then we can decide if we want to use shouldExpandReduction() to allow us to handle these reduction opcodes in dag directly

@TomHender Are you able to continue with this, otherwise would you mind if I comandeered this please?

RKSimon commandeered this revision.Feb 13 2022, 6:46 AM
RKSimon planned changes to this revision.
RKSimon edited reviewers, added: TomHender; removed: RKSimon.