This is an archive of the discontinued LLVM Phabricator instance.

[X86] Add transform for `(and/or (icmp eq/ne A,-1),(icmp eq/ne A,-1+C))`->`(and/or (icmp eq/ne (and ~A,-1+C),0))`
ClosedPublic

Authored by goldstein.w.n on Feb 17 2023, 10:32 AM.

Details

Summary

This works of -1+C is a negative power of 2.

This can be more useful than the AddAnd case as ~A does not
necessarily require materializing a constant. This makes the transform
worth it for X86 vector types.

Alive2 Links:
EQ: https://alive2.llvm.org/ce/z/P6u8cq
NE: https://alive2.llvm.org/ce/z/_Kkqp1

Diff Detail

Event Timeline

goldstein.w.n created this revision.Feb 17 2023, 10:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 17 2023, 10:32 AM
goldstein.w.n requested review of this revision.Feb 17 2023, 10:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 17 2023, 10:32 AM
goldstein.w.n added reviewers: RKSimon, pengfei.
goldstein.w.n added inline comments.
llvm/test/CodeGen/X86/icmp-pow2-dif.ll
218 ↗(On Diff #498440)

NB: This is the least useful case, but we still get vpcmpeq -> vpxor the latter of which is zero-idiom doesn't take any execution unit.

pengfei added inline comments.Feb 19 2023, 6:27 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
5962–5978

The logic here is NotAnd always precedes AddAnd, why do we need to bother bit or them?

goldstein.w.n added inline comments.Feb 19 2023, 10:14 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
5962–5978

both could be desirable transformations. Its possible for the AddAnd pattern to hit, but not the NotAnd pattern.

Comment on intentional failures

RKSimon added inline comments.Feb 20 2023, 6:17 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
5922

It should (eventually) be possible to support non-uniform constants via ISD::matchBinaryPredicate

goldstein.w.n added inline comments.Feb 20 2023, 6:19 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
5922

Is there a change here that would make that easier todo in the future? Or are you just referencing the misnamed tests?

RKSimon accepted this revision.Feb 23 2023, 6:29 PM

LGTM

llvm/include/llvm/CodeGen/TargetLowering.h
292

You might want to add description comments for each enum

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
5922

Just referencing the misnamed tests - the change itself will be pretty easy

This revision is now accepted and ready to land.Feb 23 2023, 6:29 PM

Comments for enum

goldstein.w.n marked an inline comment as done.Feb 23 2023, 7:45 PM
This revision was landed with ongoing or failed builds.Feb 24 2023, 1:23 PM
This revision was automatically updated to reflect the committed changes.