This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] isAllOnesConstantOrAllOnesSplatConstant(): look through bitcasts
ClosedPublic

Authored by lebedev.ri on May 21 2018, 12:49 PM.

Details

Summary

As pointed out in D46528, we errneously transform cases like xor X, -1,
even though we use said function.
It's because the -1 is actually a bitcast there.
So i think we can just look through it in the function.

Diff Detail

Repository
rL LLVM

Event Timeline

lebedev.ri added inline comments.May 21 2018, 12:56 PM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
916

The comment next to the ISD::BITCAST declaration notes:

/// This operator is subtly different from the bitcast instruction from
/// LLVM-IR since this node may change the bits in the register. For
/// example, this occurs on big-endian NEON and big-endian MSA where the
/// layout of the bits in the register depends on the vector type and this
/// operator acts as a shuffle operation for some vector type combinations.
BITCAST,

so i'm not too sure this is that simple,
but currently also i don't see an alternative solution.

RKSimon added inline comments.May 21 2018, 12:58 PM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
917

Don't make it recursive:

while (N.getOpcode() == ISD::BITCAST)
  N = N->getOperand(0);
lebedev.ri added inline comments.May 21 2018, 1:04 PM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
917

Oh, i can do that?
I was under impression that would change the the N itself (in IR/DAG), not just the variable N here.

lebedev.ri marked 2 inline comments as done.

No recursion.

RKSimon added inline comments.May 21 2018, 1:10 PM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
917

No, it;s fine - see ISD::isBuildVectorAllZeros - it might have been a problem if you weren't just interested in ALL bits being set.

spatel added inline comments.May 21 2018, 1:21 PM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
916

Use peekThroughBitcast()

Should we do the same for isOneConstantOrOneSplatConstant and isNullConstantOrNullSplatConstant?

RKSimon accepted this revision.May 21 2018, 1:27 PM

LGTM

lib/CodeGen/SelectionDAG/DAGCombiner.cpp
916

Let's leave it until there is a test case - you can add TODO comments if you wish.

This revision is now accepted and ready to land.May 21 2018, 1:27 PM
This revision was automatically updated to reflect the committed changes.