[llvm] c8a282b - [GlobalISel] Fix computing known bits for loads with range metadata

Jessica Paquette via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 6 16:50:19 PDT 2020


Author: Jessica Paquette
Date: 2020-08-06T16:47:07-07:00
New Revision: c8a282bcf7b6304e99f65bf22eea4553e240fb40

URL: https://github.com/llvm/llvm-project/commit/c8a282bcf7b6304e99f65bf22eea4553e240fb40
DIFF: https://github.com/llvm/llvm-project/commit/c8a282bcf7b6304e99f65bf22eea4553e240fb40.diff

LOG: [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

Added: 
    

Modified: 
    llvm/lib/Analysis/ValueTracking.cpp
    llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp

Removed: 
    


################################################################################
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());
+}


        


More information about the llvm-commits mailing list