[llvm] r355099 - Add support for computing "zext of value" in KnownBits. NFCI
Björn Pettersson A via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 4 04:06:05 PST 2019
> From: Philip Reames <listmail at philipreames.com>
> This really doesn't look right. Why would the bits added by a zext ever
> *not* be known zero?
When zero extending a bitvector with "known bits", and a zero
means that the bit isn't known. Such a zext adds zero bits
to the bitvector, that indicate that the bits are *not* known
to be zero in the tracked value.
My primary goal was to remove the confusion that the code comment
said one thing, and the code did something else.
I did not dare to simply change KnownBits::zext to always behave as
"zext of tracked value" rather than "zext of the known bits bitvectors",
since that would introduce a backwards incompatibility potentially
breaking OOT targets.
One idea is to rename these methods to computeForSext,
computeForZext (following the naming already used
for computeForAddSub). That could help out indicating
that we refer to the tracked value rather than applying
various operations on the underlying Zero/One bit vectors.
Is that preferred?
I also think that we should add "const" to these methods to
indicate that they really are const. I'm not sure why it is
like that?
Can you see a reason why we do
KnownBits = KnownBits.sext(...);
and not just
KnownBits.sext(...);
(making the methods actually update the object the operate on)?
Regards,
Björn
> -----Original Message-----
> From: Philip Reames <listmail at philipreames.com>
> Sent: den 1 mars 2019 21:13
> To: Björn Pettersson A <bjorn.a.pettersson at ericsson.com>; llvm-
> commits at lists.llvm.org
> Subject: Re: [llvm] r355099 - Add support for computing "zext of value" in
> KnownBits. NFCI
>
> This really doesn't look right. Why would the bits added by a zext ever
> *not* be known zero?
>
> On 2/28/19 7:45 AM, Bjorn Pettersson via llvm-commits wrote:
> > Author: bjope
> > Date: Thu Feb 28 07:45:29 2019
> > New Revision: 355099
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=355099&view=rev
> > Log:
> > Add support for computing "zext of value" in KnownBits. NFCI
> >
> > Summary:
> > The description of KnownBits::zext() and
> > KnownBits::zextOrTrunc() has confusingly been telling
> > that the operation is equivalent to zero extending the
> > value we're tracking. That has not been true, instead
> > the user has been forced to explicitly set the extended
> > bits as known zero afterwards.
> >
> > This patch adds a second argument to KnownBits::zext()
> > and KnownBits::zextOrTrunc() to control if the extended
> > bits should be considered as known zero or as unknown.
> >
> > Reviewers: craig.topper, RKSimon
> >
> > Reviewed By: RKSimon
> >
> > Subscribers: javed.absar, hiraditya, jdoerfert, llvm-commits
> >
> > Tags: #llvm
> >
> > Differential Revision: https://reviews.llvm.org/D58650
> >
> > Modified:
> > llvm/trunk/include/llvm/Support/KnownBits.h
> > llvm/trunk/lib/Analysis/ValueTracking.cpp
> > llvm/trunk/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
> > llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> > llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp
> > llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
> > llvm/trunk/lib/Target/SystemZ/SystemZISelLowering.cpp
> > llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> > llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
> >
> > Modified: llvm/trunk/include/llvm/Support/KnownBits.h
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/include/llvm/Support/KnownBits.h?rev=355099&r1=355098&r2
> =355099&view=diff
> >
> ===========================================================================
> ===
> > --- llvm/trunk/include/llvm/Support/KnownBits.h (original)
> > +++ llvm/trunk/include/llvm/Support/KnownBits.h Thu Feb 28 07:45:29 2019
> > @@ -113,10 +113,16 @@ public:
> > return KnownBits(Zero.trunc(BitWidth), One.trunc(BitWidth));
> > }
> >
> > - /// Zero extends the underlying known Zero and One bits. This is
> equivalent
> > - /// to zero extending the value we're tracking.
> > - KnownBits zext(unsigned BitWidth) {
> > - return KnownBits(Zero.zext(BitWidth), One.zext(BitWidth));
> > + /// Extends the underlying known Zero and One bits.
> > + /// By setting ExtendedBitsAreKnownZero=true this will be equivalent
> to
> > + /// zero extending the value we're tracking.
> > + /// With ExtendedBitsAreKnownZero=false the extended bits are set to
> unknown.
> > + KnownBits zext(unsigned BitWidth, bool ExtendedBitsAreKnownZero) {
> > + unsigned OldBitWidth = getBitWidth();
> > + APInt NewZero = Zero.zext(BitWidth);
> > + if (ExtendedBitsAreKnownZero)
> > + NewZero.setBitsFrom(OldBitWidth);
> > + return KnownBits(NewZero, One.zext(BitWidth));
> > }
> >
> > /// Sign extends the underlying known Zero and One bits. This is
> equivalent
> > @@ -125,9 +131,13 @@ public:
> > return KnownBits(Zero.sext(BitWidth), One.sext(BitWidth));
> > }
> >
> > - /// Zero extends or truncates the underlying known Zero and One bits.
> This is
> > - /// equivalent to zero extending or truncating the value we're
> tracking.
> > - KnownBits zextOrTrunc(unsigned BitWidth) {
> > + /// Extends or truncates the underlying known Zero and One bits. When
> > + /// extending the extended bits can either be set as known zero (if
> > + /// ExtendedBitsAreKnownZero=true) or as unknown (if
> > + /// ExtendedBitsAreKnownZero=false).
> > + KnownBits zextOrTrunc(unsigned BitWidth, bool
> ExtendedBitsAreKnownZero) {
> > + if (BitWidth > getBitWidth())
> > + return zext(BitWidth, ExtendedBitsAreKnownZero);
> > return KnownBits(Zero.zextOrTrunc(BitWidth),
> One.zextOrTrunc(BitWidth));
> > }
> >
> >
> > Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=355099&r1=355098&r2=3
> 55099&view=diff
> >
> ===========================================================================
> ===
> > --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
> > +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Thu Feb 28 07:45:29 2019
> > @@ -1128,12 +1128,9 @@ static void computeKnownBitsFromOperator
> > Q.DL.getTypeSizeInBits(ScalarTy);
> >
> > assert(SrcBitWidth && "SrcBitWidth can't be zero");
> > - Known = Known.zextOrTrunc(SrcBitWidth);
> > + Known = Known.zextOrTrunc(SrcBitWidth, false);
> > computeKnownBits(I->getOperand(0), Known, Depth + 1, Q);
> > - Known = Known.zextOrTrunc(BitWidth);
> > - // Any top bits are known to be zero.
> > - if (BitWidth > SrcBitWidth)
> > - Known.Zero.setBitsFrom(SrcBitWidth);
> > + Known = Known.zextOrTrunc(BitWidth, true /* ExtendedBitsAreKnownZero
> */);
> > break;
> > }
> > case Instruction::BitCast: {
> >
> > Modified: llvm/trunk/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp?rev=35
> 5099&r1=355098&r2=355099&view=diff
> >
> ===========================================================================
> ===
> > --- llvm/trunk/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
> (original)
> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp Thu Feb
> 28 07:45:29 2019
> > @@ -400,7 +400,7 @@ FunctionLoweringInfo::GetLiveOutRegInfo(
> >
> > if (BitWidth > LOI->Known.getBitWidth()) {
> > LOI->NumSignBits = 1;
> > - LOI->Known = LOI->Known.zextOrTrunc(BitWidth);
> > + LOI->Known = LOI->Known.zext(BitWidth, false /* => any extend */);
> > }
> >
> > return LOI;
> >
> > Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=355099&r1=
> 355098&r2=355099&view=diff
> >
> ===========================================================================
> ===
> > --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Thu Feb 28
> 07:45:29 2019
> > @@ -2800,15 +2800,12 @@ KnownBits SelectionDAG::computeKnownBits
> > EVT InVT = Op.getOperand(0).getValueType();
> > APInt InDemandedElts =
> DemandedElts.zextOrSelf(InVT.getVectorNumElements());
> > Known = computeKnownBits(Op.getOperand(0), InDemandedElts, Depth +
> 1);
> > - Known = Known.zext(BitWidth);
> > - Known.Zero.setBitsFrom(InVT.getScalarSizeInBits());
> > + Known = Known.zext(BitWidth, true /* ExtendedBitsAreKnownZero */);
> > break;
> > }
> > case ISD::ZERO_EXTEND: {
> > - EVT InVT = Op.getOperand(0).getValueType();
> > Known = computeKnownBits(Op.getOperand(0), DemandedElts, Depth + 1);
> > - Known = Known.zext(BitWidth);
> > - Known.Zero.setBitsFrom(InVT.getScalarSizeInBits());
> > + Known = Known.zext(BitWidth, true /* ExtendedBitsAreKnownZero */);
> > break;
> > }
> > case ISD::SIGN_EXTEND_VECTOR_INREG: {
> > @@ -2829,7 +2826,7 @@ KnownBits SelectionDAG::computeKnownBits
> > }
> > case ISD::ANY_EXTEND: {
> > Known = computeKnownBits(Op.getOperand(0), Depth+1);
> > - Known = Known.zext(BitWidth);
> > + Known = Known.zext(BitWidth, false /* ExtendedBitsAreKnownZero */);
> > break;
> > }
> > case ISD::TRUNCATE: {
> > @@ -3026,7 +3023,7 @@ KnownBits SelectionDAG::computeKnownBits
> > Known = computeKnownBits(InVec, Depth + 1);
> > }
> > if (BitWidth > EltBitWidth)
> > - Known = Known.zext(BitWidth);
> > + Known = Known.zext(BitWidth, false /* => any extend */);
> > break;
> > }
> > case ISD::INSERT_VECTOR_ELT: {
> >
> > Modified: llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp?rev=355099&r
> 1=355098&r2=355099&view=diff
> >
> ===========================================================================
> ===
> > --- llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp Thu Feb 28
> 07:45:29 2019
> > @@ -1163,8 +1163,8 @@ bool TargetLowering::SimplifyDemandedBit
> > if (SimplifyDemandedBits(Src, InDemandedBits, Known, TLO, Depth +
> 1))
> > return true;
> > assert(!Known.hasConflict() && "Bits known to be one AND zero?");
> > - Known = Known.zext(BitWidth);
> > - Known.Zero.setBitsFrom(InBits);
> > + assert(Known.getBitWidth() == InBits && "Src width has changed?");
> > + Known = Known.zext(BitWidth, true /* ExtendedBitsAreKnownZero */);
> > break;
> > }
> > case ISD::SIGN_EXTEND: {
> > @@ -1217,7 +1217,7 @@ bool TargetLowering::SimplifyDemandedBit
> > if (SimplifyDemandedBits(Src, InDemandedBits, Known, TLO, Depth +
> 1))
> > return true;
> > assert(!Known.hasConflict() && "Bits known to be one AND zero?");
> > - Known = Known.zext(BitWidth);
> > + Known = Known.zext(BitWidth, false /* => any extend */);
> > break;
> > }
> > case ISD::TRUNCATE: {
> > @@ -1312,7 +1312,7 @@ bool TargetLowering::SimplifyDemandedBit
> >
> > Known = Known2;
> > if (BitWidth > EltBitWidth)
> > - Known = Known.zext(BitWidth);
> > + Known = Known.zext(BitWidth, false /* => any extend */);
> > break;
> > }
> > case ISD::BITCAST: {
> >
> > Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=355099&r1=355098&
> r2=355099&view=diff
> >
> ===========================================================================
> ===
> > --- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)
> > +++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Thu Feb 28 07:45:29
> 2019
> > @@ -13706,8 +13706,7 @@ void ARMTargetLowering::computeKnownBits
> > if (Op.getOpcode() == ARMISD::VGETLANEs)
> > Known = Known.sext(DstSz);
> > else {
> > - Known = Known.zext(DstSz);
> > - Known.Zero.setBitsFrom(SrcSz);
> > + Known = Known.zext(DstSz, true /* extended bits are known zero
> */);
> > }
> > assert(DstSz == Known.getBitWidth());
> > break;
> >
> > Modified: llvm/trunk/lib/Target/SystemZ/SystemZISelLowering.cpp
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Target/SystemZ/SystemZISelLowering.cpp?rev=355099&r1
> =355098&r2=355099&view=diff
> >
> ===========================================================================
> ===
> > --- llvm/trunk/lib/Target/SystemZ/SystemZISelLowering.cpp (original)
> > +++ llvm/trunk/lib/Target/SystemZ/SystemZISelLowering.cpp Thu Feb 28
> 07:45:29 2019
> > @@ -6053,12 +6053,10 @@ SystemZTargetLowering::computeKnownBitsF
> > case Intrinsic::s390_vuplhw:
> > case Intrinsic::s390_vuplf: {
> > SDValue SrcOp = Op.getOperand(1);
> > - unsigned SrcBitWidth = SrcOp.getScalarValueSizeInBits();
> > APInt SrcDemE = getDemandedSrcElements(Op, DemandedElts, 0);
> > Known = DAG.computeKnownBits(SrcOp, SrcDemE, Depth + 1);
> > if (IsLogical) {
> > - Known = Known.zext(BitWidth);
> > - Known.Zero.setBitsFrom(SrcBitWidth);
> > + Known = Known.zext(BitWidth, true);
> > } else
> > Known = Known.sext(BitWidth);
> > break;
> > @@ -6087,7 +6085,7 @@ SystemZTargetLowering::computeKnownBitsF
> > // Known has the width of the source operand(s). Adjust if needed to
> match
> > // the passed bitwidth.
> > if (Known.getBitWidth() != BitWidth)
> > - Known = Known.zextOrTrunc(BitWidth);
> > + Known = Known.zextOrTrunc(BitWidth, false);
> > }
> >
> > static unsigned computeNumSignBitsBinOp(SDValue Op, const APInt
> &DemandedElts,
> >
> > Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=355099&r1=355098&
> r2=355099&view=diff
> >
> ===========================================================================
> ===
> > --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> > +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Thu Feb 28 07:45:29
> 2019
> > @@ -30412,7 +30412,7 @@ void X86TargetLowering::computeKnownBits
> > APInt DemandedElt =
> APInt::getOneBitSet(SrcVT.getVectorNumElements(),
> >
> Op.getConstantOperandVal(1));
> > Known = DAG.computeKnownBits(Src, DemandedElt, Depth + 1);
> > - Known = Known.zextOrTrunc(BitWidth);
> > + Known = Known.zextOrTrunc(BitWidth, false);
> > Known.Zero.setBitsFrom(SrcVT.getScalarSizeInBits());
> > break;
> > }
> >
> > Modified:
> llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.c
> pp?rev=355099&r1=355098&r2=355099&view=diff
> >
> ===========================================================================
> ===
> > --- llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
> (original)
> > +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
> Thu Feb 28 07:45:29 2019
> > @@ -365,10 +365,9 @@ Value *InstCombiner::SimplifyDemandedUse
> > KnownBits InputKnown(SrcBitWidth);
> > if (SimplifyDemandedBits(I, 0, InputDemandedMask, InputKnown, Depth
> + 1))
> > return I;
> > - Known = InputKnown.zextOrTrunc(BitWidth);
> > - // Any top bits are known to be zero.
> > - if (BitWidth > SrcBitWidth)
> > - Known.Zero.setBitsFrom(SrcBitWidth);
> > + assert(InputKnown.getBitWidth() == SrcBitWidth && "Src width
> changed?");
> > + Known = InputKnown.zextOrTrunc(BitWidth,
> > + true /* ExtendedBitsAreKnownZero */);
> > assert(!Known.hasConflict() && "Bits known to be one AND zero?");
> > break;
> > }
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list