Skip to content

[DAG] Fold (and X, (bswap/bitreverse (not Y))) -> (and X, (not (bswap/bitreverse Y))) #112547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 28, 2024
Merged

[DAG] Fold (and X, (bswap/bitreverse (not Y))) -> (and X, (not (bswap/bitreverse Y))) #112547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b8996f8
[DAG] Fold (and X, (bswap/bitreverse (not Y))) -> (and X, (not (bswap…
RKSimon Oct 16, 2024
9ba8f81
[X86] andnot-patterns.ll - add additional multiuse tests
RKSimon Oct 18, 2024
77adbb3
Relax ANDNOT requirement to be only for multiple uses of the NOT
RKSimon Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7353,6 +7353,17 @@ SDValue DAGCombiner::visitAND(SDNode *N) {
if (SDValue R = foldLogicOfShifts(N, N1, N0, DAG))
return R;

// Fold (and X, (bswap (not Y))) -> (and X, (not (bswap Y)))
// Fold (and X, (bitreverse (not Y))) -> (and X, (not (bitreverse Y)))
SDValue X, Y, NotY;
for (unsigned Opc : {ISD::BSWAP, ISD::BITREVERSE})
if (sd_match(N,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe should be || (NotY->hasOneUse() && sd_match(X, m_Not(m_Value()))

There is a lot of diff from just re-assosiating the not which seems netural.

No strong opinion, however.

m_And(m_Value(X), m_OneUse(m_UnaryOp(Opc, m_Value(NotY))))) &&
sd_match(NotY, m_Not(m_Value(Y))) &&
(TLI.hasAndNot(SDValue(N, 0)) || NotY->hasOneUse()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? Seems like it expose ~X & ~Y -> ~(X | Y)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is ~X & bitreverse/bswap(~Y).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - its to prevent the fold when we already have a ANDNOT pattern.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean isn't ~X & bitreverse/bswap(~Y) -> ~X & ~bitreverse/bswap(Y) also profitable? Irrelivant of ANDNOT in fact.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess assuming ~Y is one-use.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for the NOT(Y) we can allow multiple uses as with a ANDNOT we'll be getting it for free - I'd be willing to allow this fold to occur on targets without ANDNOT, if for those targets we require the NOT(Y) to have one use?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, something like

if (sd_match(N, m_And(m_Value(X),
                            m_OneUse(m_UnaryOp(Opc, m_Value(NotY)))) &&
   match(NotY, m_Not(m_Value(Y))) && (TLI.hasAndNot(SDValue(N, 0) || NotY->hasOneUse())

should work.

return DAG.getNode(ISD::AND, DL, VT, X,
DAG.getNOT(DL, DAG.getNode(Opc, DL, VT, Y), VT));

// Masking the negated extension of a boolean is just the zero-extended
// boolean:
// and (sub 0, zext(bool X)), 1 --> zext(bool X)
Expand Down
Loading
Loading