This is an archive of the discontinued LLVM Phabricator instance.

[GlobalIsel][x86] Legalize G_CTPOP and G_CTLZ II
ClosedPublic

Authored by tschuett on May 17 2023, 12:08 AM.

Details

Summary

check plan: ninja check-llvm-codegen-x86

Diff Detail

Event Timeline

tschuett created this revision.May 17 2023, 12:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2023, 12:08 AM
tschuett requested review of this revision.May 17 2023, 12:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2023, 12:08 AM
tschuett updated this revision to Diff 522922.May 17 2023, 12:11 AM

add missing newlines

arsenm accepted this revision.May 17 2023, 3:32 AM
This revision is now accepted and ready to land.May 17 2023, 3:32 AM
This revision was landed with ongoing or failed builds.May 17 2023, 6:02 AM
This revision was automatically updated to reflect the committed changes.
RKSimon added inline comments.
llvm/lib/Target/X86/X86LegalizerInfo.cpp
389

This isn't correct - CTPOP/LZCNT support is provided by Subtarget.hasPOPCNT/hasLZCNT respectively

I was also unsure about the availability, but Wikipedia claims:
https://en.wikipedia.org/wiki/X86_Bit_manipulation_instruction_set

ABM is implemented by SSE4.2 and ABM is popcount and lzcnt.

I was also unsure about the availability, but Wikipedia claims:
https://en.wikipedia.org/wiki/X86_Bit_manipulation_instruction_set

ABM is implemented by SSE4.2 and ABM is popcount and lzcnt.

It should be possible to disable the generation of these instructions by passing -mno-popcnt / -mno-lzcnt frontend option even if SSE4.2 is enabled.

RKSimon added inline comments.May 17 2023, 7:56 AM
llvm/lib/Target/X86/X86LegalizerInfo.cpp
394

Additionally, don't we need to guard against 32-bit triples where s64 isn't legal?

I was also unsure about the availability, but Wikipedia claims:
https://en.wikipedia.org/wiki/X86_Bit_manipulation_instruction_set

ABM is implemented by SSE4.2 and ABM is popcount and lzcnt.

Don't trust wikipedia - it might be that all SSE4.2 cpus are known to support ABM as well, but its not under the same feature flags inside llvm (or the same cpuid bits on actual cpus), and AMD in fact support it with older cpus.

I am on it. It will take a moment.