Page MenuHomePhabricator

[X86] Restore the pavg intrinsics.
ClosedPublic

Authored by craig.topper on Sun, Apr 14, 6:12 PM.

Details

Summary

The pattern we replaced these with may be too hard to match as demonstrated by
PR41496 and PR41316.

This patch proposes to restore the intrinsics and then we can start focusing
on the optimizing the intrinsics.

I've mostly reverted the original patch that removed them. Though I modified
the avx512 intrinsics to not have masking built in.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Sun, Apr 14, 6:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptSun, Apr 14, 6:12 PM
Herald added a subscriber: hiraditya. · View Herald Transcript

Add the clang changes

Herald added a project: Restricted Project. · View Herald TranscriptSun, Apr 14, 7:39 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
nikic added a subscriber: nikic.Mon, Apr 15, 12:13 AM
RKSimon accepted this revision.Mon, Apr 15, 4:39 AM

LGTM - there's too many different optimizations and canonicalizations that can occur on such a pattern to be able to match all of the permutations.

We can probably add some InstCombine optimizations (e.g. where the avg can't overflow etc.) and keep the existing DAG matching.

This revision is now accepted and ready to land.Mon, Apr 15, 4:39 AM
Closed by commit rL358427: [X86] Restore the pavg intrinsics. (authored by ctopper, committed by ). · Explain WhyMon, Apr 15, 10:16 AM
This revision was automatically updated to reflect the committed changes.

Though I modified the avx512 intrinsics to not have masking built in.

Do we need autoupgrade support from the old avx512 intrinsics to the new avx512 intrinsics?

Though I modified the avx512 intrinsics to not have masking built in.

Do we need autoupgrade support from the old avx512 intrinsics to the new avx512 intrinsics?

Yes, and the code at 1548 in AutoUpgrade.cpp does that. There are existing tests in avx512bw-intrinsics-upgrade.ll and avx512bwvl-intrinsics-upgrade.ll that were previously testing upgrade from old intrinsics to pure IR that now just test old intrinsics to new intrinsics. The codegen is the same so they weren't affected by the patch.

Okay, thanks.