This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Remaining KnownBits should be constant when performing non-sign comparison
ClosedPublic

Authored by lkail on Dec 11 2020, 12:45 AM.

Details

Summary

In PPCTargetLowering::DAGCombineTruncBoolExt, when checking if it's correct to perform the transformation for non-sign comparison, as the comment says

// This is neither a signed nor an unsigned comparison, just make sure                                                                                                                  
// that the high bits are equal.

Origin check

if (Op1Known.Zero != Op2Known.Zero || Op1Known.One != Op2Known.One)
  return SDValue();

is not strong enough. For example,

Op1Known = 111x000x;
Op2Known = 111x000x;

Bit 4, besides bit 0, is still unknown and affects the final result.

This patch fixes https://bugs.llvm.org/show_bug.cgi?id=48388.

Diff Detail

Event Timeline

lkail created this revision.Dec 11 2020, 12:45 AM
lkail requested review of this revision.Dec 11 2020, 12:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 11 2020, 12:45 AM
nemanjai added inline comments.Dec 29 2020, 3:49 AM
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
13245–13246

Hmm, but we've cleared bit zero from both known zero and known ones for both. So they will both be non-constant, won't they? I think that we would have to rewrite the above lines to something like this:

// We don't really care about what is known about the first bit (if
// anything), so pretend that it is known zero for both to ensure
// they can be compared as constants.
Op1Known.Zero.setBit(0); Op1Known.One.clearBit(0);
Op2Known.Zero.setBit(0); Op2Known.One.clearBit(0);
lkail added inline comments.Dec 29 2020, 4:08 AM
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
13245–13246

Good catch! This check is over kill, since we are requiring Op1[1..BitWidth) == Op2[1..BitWidth). I tend to extract bits in [1..BitWidth) from Op1 and Op2 and do same constant check as above.

lkail updated this revision to Diff 313961.Dec 29 2020, 4:36 AM

@nemanjai 's solution can make code more compact. Thanks.

lkail updated this revision to Diff 313962.Dec 29 2020, 4:38 AM
lkail retitled this revision from [PowerPC] KnownBits should be constant when performing non-sign comparison to [PowerPC] Remaining KnownBits should be constant when performing non-sign comparison.
nemanjai accepted this revision.Dec 29 2020, 4:54 AM

LGTM. Thanks.

This revision is now accepted and ready to land.Dec 29 2020, 4:54 AM
This revision was landed with ongoing or failed builds.Dec 29 2020, 6:00 PM
This revision was automatically updated to reflect the committed changes.