[PATCH] D134380: [GlobalISel] Fix known bits for G_ASSERT_ALIGN.

[Amara Emerson via Phabricator via llvm-commits]

aemerson created this revision.
aemerson added reviewers: arsenm, paquette, foad.
aemerson added a project: LLVM.
Herald added subscribers: hiraditya, rovka.
Herald added a project: All.
aemerson requested review of this revision.
Herald added a subscriber: wdng.

I don't know what was going on originally with these tests. It seems reasonable to have the immediate be the same byte alignment unit as the IR, in which case we need to take the log2 in order to set the right number of low bits.

This fixes a miscompile in chromium.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134380

Files:
  llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
  llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp


Index: llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp
===================================================================
--- llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp
+++ llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp
@@ -1932,17 +1932,14 @@
    %assert_align2:_(s64) = G_ASSERT_ALIGN %val, 2
    %copy_assert_align2:_(s64) = COPY %assert_align2
 
-   %assert_align3:_(s64) = G_ASSERT_ALIGN %val, 3
-   %copy_assert_align3:_(s64) = COPY %assert_align3
+   %assert_align4:_(s64) = G_ASSERT_ALIGN %val, 4
+   %copy_assert_align4:_(s64) = COPY %assert_align4
 
    %assert_align8:_(s64) = G_ASSERT_ALIGN %val, 8
    %copy_assert_align8:_(s64) = COPY %assert_align8
 
-   %assert_maxalign:_(s64) = G_ASSERT_ALIGN %val, 30
-   %copy_assert_maxalign:_(s64) = COPY %assert_maxalign
-
-   %assert_ptr_align5:_(p1) = G_ASSERT_ALIGN %ptrval, 5
-   %copy_assert_ptr_align5:_(p1) = COPY %assert_ptr_align5
+   %assert_align16:_(s64) = G_ASSERT_ALIGN %val, 16
+   %copy_assert_maxalign:_(s64) = COPY %assert_align16
 )MIR";
   setUp(MIRString);
   if (!TM)
@@ -1959,18 +1956,23 @@
   auto CheckBits = [&](unsigned NumBits, unsigned Idx) {
     Res = GetKB(Idx);
     EXPECT_EQ(64u, Res.getBitWidth());
-    EXPECT_EQ(NumBits, Res.Zero.countTrailingOnes());
+    EXPECT_EQ(NumBits - 1, Res.Zero.countTrailingOnes());
     EXPECT_EQ(64u, Res.One.countTrailingZeros());
-    EXPECT_EQ(Align(1ull << NumBits), Info.computeKnownAlignment(Copies[Idx]));
+    EXPECT_EQ(Align(1ull << (NumBits - 1)), Info.computeKnownAlignment(Copies[Idx]));
   };
 
-  CheckBits(0, Copies.size() - 7);
-  CheckBits(1, Copies.size() - 6);
-  CheckBits(2, Copies.size() - 5);
-  CheckBits(3, Copies.size() - 4);
-  CheckBits(8, Copies.size() - 3);
-  CheckBits(30, Copies.size() - 2);
-  CheckBits(5, Copies.size() - 1);
+  const unsigned NumSetupCopies = 5;
+  // Check zero align specially.
+  Res = GetKB(NumSetupCopies);
+  EXPECT_EQ(0u, Res.Zero.countTrailingOnes());
+  EXPECT_EQ(64u, Res.One.countTrailingZeros());
+  EXPECT_EQ(Align(1), Info.computeKnownAlignment(Copies[5]));
+
+  CheckBits(1, NumSetupCopies + 1);
+  CheckBits(2, NumSetupCopies + 2);
+  CheckBits(3, NumSetupCopies + 3);
+  CheckBits(4, NumSetupCopies + 4);
+  CheckBits(5, NumSetupCopies + 5);
 }
 
 TEST_F(AArch64GISelMITest, TestKnownBitsUADDO) {
Index: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
===================================================================
--- llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
+++ llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
@@ -39,8 +39,7 @@
     return computeKnownAlignment(MI->getOperand(1).getReg(), Depth);
   case TargetOpcode::G_ASSERT_ALIGN: {
     // TODO: Min with source
-    int64_t LogAlign = MI->getOperand(2).getImm();
-    return Align(1ull << LogAlign);
+    return Align(std::max(1LL, MI->getOperand(2).getImm()));
   }
   case TargetOpcode::G_FRAME_INDEX: {
     int FrameIdx = MI->getOperand(1).getIndex();
@@ -472,9 +471,10 @@
     break;
   }
   case TargetOpcode::G_ASSERT_ALIGN: {
-    int64_t LogOfAlign = MI.getOperand(2).getImm();
-    if (LogOfAlign == 0)
+    int64_t Align = MI.getOperand(2).getImm();
+    if (Align == 0)
       break;
+    int64_t LogOfAlign = Log2_64(Align);
 
     // TODO: Should use maximum with source
     // If a node is guaranteed to be aligned, set low zero bits accordingly as


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D134380.461988.patch
Type: text/x-patch
Size: 3346 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220921/59d36ba0/attachment.bin>