From c8a282bcf7b6304e99f65bf22eea4553e240fb40 Mon Sep 17 00:00:00 2001 From: Jessica Paquette Date: Wed, 5 Aug 2020 16:32:55 -0700 Subject: [PATCH] [GlobalISel] Fix computing known bits for loads with range metadata In GlobalISel, if you have a load into a small type with a range, you'll hit an assert if you try to compute known bits on it starting at a larger type. e.g. ``` %x:_(s8) = G_LOAD %whatever(p0) :: (load 1 ... !range !n) ... %y:_(s32) = G_SOMETHING %x ``` When we walk through G_SOMETHING and hit the load, the width of our known bits is 32. However, the width of the range is going to be 8. This will cause us to hit an assert. To fix this, make computeKnownBitsFromRangeMetadata zero extend or truncate the range type to match the bitwidth of the known bits we're calculating. Add a testcase in CodeGen/GlobalISel/KnownBitsTest.cpp to reflect that this works now. https://reviews.llvm.org/D85375 --- llvm/lib/Analysis/ValueTracking.cpp | 6 +-- .../CodeGen/GlobalISel/KnownBitsTest.cpp | 48 +++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp index af55c96f3a53..3cd90e8ece75 100644 --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -549,10 +549,10 @@ void llvm::computeKnownBitsFromRangeMetadata(const MDNode &Ranges, // The first CommonPrefixBits of all values in Range are equal. unsigned CommonPrefixBits = (Range.getUnsignedMax() ^ Range.getUnsignedMin()).countLeadingZeros(); - APInt Mask = APInt::getHighBitsSet(BitWidth, CommonPrefixBits); - Known.One &= Range.getUnsignedMax() & Mask; - Known.Zero &= ~Range.getUnsignedMax() & Mask; + APInt UnsignedMax = Range.getUnsignedMax().zextOrTrunc(BitWidth); + Known.One &= UnsignedMax & Mask; + Known.Zero &= ~UnsignedMax & Mask; } } diff --git a/llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp b/llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp index 329c8040158d..9e231bed11ab 100644 --- a/llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp +++ b/llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp @@ -463,3 +463,51 @@ TEST_F(AMDGPUGISelMITest, TestTargetKnownAlign) { EXPECT_EQ(Align(4), Info.computeKnownAlignment(CopyImplicitArgPtr)); EXPECT_EQ(Align(4), Info.computeKnownAlignment(CopyImplicitBufferPtr)); } + +TEST_F(AArch64GISelMITest, TestMetadata) { + StringRef MIRString = " %imp:_(p0) = G_IMPLICIT_DEF\n" + " %load:_(s8) = G_LOAD %imp(p0) :: (load 1)\n" + " %ext:_(s32) = G_ZEXT %load(s8)\n" + " %cst:_(s32) = G_CONSTANT i32 1\n" + " %and:_(s32) = G_AND %ext, %cst\n" + " %copy:_(s32) = COPY %and(s32)\n"; + setUp(MIRString); + if (!TM) + return; + + Register CopyReg = Copies[Copies.size() - 1]; + MachineInstr *FinalCopy = MRI->getVRegDef(CopyReg); + Register SrcReg = FinalCopy->getOperand(1).getReg(); + + // We need a load with a metadata range for this to break. Fudge the load in + // the string and replace it with something we can work with. + MachineInstr *And = MRI->getVRegDef(SrcReg); + MachineInstr *Ext = MRI->getVRegDef(And->getOperand(1).getReg()); + MachineInstr *Load = MRI->getVRegDef(Ext->getOperand(1).getReg()); + IntegerType *Int8Ty = Type::getInt8Ty(Context); + + // Value must be in [0, 2) + Metadata *LowAndHigh[] = { + ConstantAsMetadata::get(ConstantInt::get(Int8Ty, 0)), + ConstantAsMetadata::get(ConstantInt::get(Int8Ty, 2))}; + auto NewMDNode = MDNode::get(Context, LowAndHigh); + const MachineMemOperand *OldMMO = *Load->memoperands_begin(); + MachineMemOperand NewMMO(OldMMO->getPointerInfo(), OldMMO->getFlags(), + OldMMO->getSizeInBits(), OldMMO->getAlign(), + OldMMO->getAAInfo(), NewMDNode); + MachineIRBuilder MIB(*Load); + MIB.buildLoad(Load->getOperand(0), Load->getOperand(1), NewMMO); + Load->eraseFromParent(); + + GISelKnownBits Info(*MF); + KnownBits Res = Info.getKnownBits(And->getOperand(1).getReg()); + + // We don't know what the result of the load is, so we don't know any ones. + EXPECT_TRUE(Res.One.isNullValue()); + + // We know that the value is in [0, 2). So, we don't know if the first bit + // is 0 or not. However, we do know that every other bit must be 0. + APInt Mask(Res.getBitWidth(), 1); + Mask.flipAllBits(); + EXPECT_EQ(Mask.getZExtValue(), Res.Zero.getZExtValue()); +}