[PATCH] D77201: [CodeGen][SelectionDAG] Flip Booleans More Often

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added a comment.

In D77201#1955664 <https://reviews.llvm.org/D77201#1955664>, @efriedma wrote:

> We have to call getBooleanContents on the vector type, not the vector element type.  On ARM, a boolean in a vector is ZeroOrNegativeOneBooleanContent.  A scalar boolean on its own is ZeroOrOneBooleanContent.  If you choose incorrectly, that's a potential miscompile.  (Not sure why it isn't showing up as a regression test change for other targets; maybe the specific VSELECT pattern in question is rare after legalization.)
>
> Maybe the code should be truncating the APInt before it checks isAllOnesValue(), though.  For a boolean, isOne() and isAllOnesValue() should be equivalent.


Thank you for this explanation. Is the kind of change you have in mind? (I tested it and it also works)

  ConstantSDNode *Const = isConstOrConstSplat(V.getOperand(1),
                                              /*AllowUndefs*/ false,
                                              /*AllowTruncation*/ true);
  if (!Const)
    return SDValue();
  EVT VT = V.getValueType();
  
  bool IsFlip = false;
  switch(TLI.getBooleanContents(VT)) {
    case TargetLowering::ZeroOrOneBooleanContent:
      IsFlip = Const->isOne();
      break;
    case TargetLowering::ZeroOrNegativeOneBooleanContent:
      IsFlip =  Const->getAPIntValue()
                  .sextOrTrunc(Const->getNumValues())
                  .isAllOnesValue();
      break;
    case TargetLowering::UndefinedBooleanContent:
      IsFlip = (Const->getAPIntValue() & 0x01) == 1;
      break;
  }


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77201/new/

https://reviews.llvm.org/D77201