[unittests] demonstrate User::replaceUsesOfWith() breaking CallBrInst

[nickdesaulniers]

Summary

Also, refactor CallBrInstruction IRTest. Run via:

$ ninja IRTests
$ ./unittests/IR/IRTests --gtest_filter=InstructionsTest.CallBrInstruction

Link: https://github.com/ClangBuiltLinux/linux/issues/896

[nickdesaulniers]

whoops, didn't mean to commit all that, let me drop some of this.

[nickdesaulniers]

• drop mistakenly committed hacked up files

[nickdesaulniers]

• git-clang-format HEAD~2

[nickdesaulniers]

Can't land this test as is, because it would be red, due to User::replaceUsesOfWith() not updating the asm's argparam.

[nickdesaulniers]

• I hate phab

[nickdesaulniers]

F!#$ I updated the wrong phab link!!!!!!!!!!!

[nickdesaulniers]

• restore to rebased version

[nickdesaulniers]

This is no longer an issue after https://github.com/llvm/llvm-project/commit/2a721374aef3 since we now use !i placeholders to denote that these inputs are implicit; they're no longer listed twice in the operand list.

llvm/lib/IR/User.cpp

 //===-- User.cpp - Implement the User class -------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/User.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/GlobalValue.h"
 
+#include "llvm/Support/raw_ostream.h" //
+
 namespace llvm {
 class BasicBlock;
 
 //===----------------------------------------------------------------------===//
 //                                 User Class
 //===----------------------------------------------------------------------===//
 
 void User::replaceUsesOfWith(Value *From, Value *To) {
   if (From == To) return;   // Duh what?
 
   assert((!isa<Constant>(this) || isa<GlobalValue>(this)) &&
          "Cannot call User::replaceUsesOfWith on a constant!");
 
-  for (unsigned i = 0, E = getNumOperands(); i != E; ++i)
+  for (unsigned i = 0, E = getNumOperands(); i != E; ++i) {
+    //llvm::errs() << "op " << i << ": " << *getOperand(i) << "\n";
     if (getOperand(i) == From) {  // Is This operand is pointing to oldval?
       // The side effects of this setOperand call include linking to
       // "To", adding "this" to the uses list of To, and
       // most importantly, removing "this" from the use list of "From".
       setOperand(i, To); // Fix it now...
-    }
+      //llvm::errs() << __func__ << ": " << *From << " -> " << *To << "\n";
+    }
 }
 
 //===----------------------------------------------------------------------===//
 //                         User allocHungoffUses Implementation
 //===----------------------------------------------------------------------===//
 
 void User::allocHungoffUses(unsigned N, bool IsPhi) {
   assert(HasHungOffUses && "alloc must have hung off uses");

llvm/lib/Transforms/Utils/BasicBlockUtils.cpp

if (LI && LI->isLoopHeader(BB))
     // Using the loop start line number prevents debuggers stepping into the
     // loop body for this instruction.
     BI->setDebugLoc(LI->getLoopFor(BB)->getStartLoc());
   else
     BI->setDebugLoc(BB->getFirstNonPHIOrDbg()->getDebugLoc());
 
   // Move the edges from Preds to point to NewBB instead of BB.
   for (unsigned i = 0, e = Preds.size(); i != e; ++i) {
+    Instruction* T = Preds[i]->getTerminator();
     // This is slightly more strict than necessary; the minimum requirement
     // is that there be no more than one indirectbr branching to BB. And
     // all BlockAddress uses would need to be updated.
-    assert(!isa<IndirectBrInst>(Preds[i]->getTerminator()) &&
+    assert(!isa<IndirectBrInst>(T) &&
            "Cannot split an edge from an IndirectBrInst");
-    assert(!isa<CallBrInst>(Preds[i]->getTerminator()) &&
-           "Cannot split an edge from a CallBrInst");
-    Preds[i]->getTerminator()->replaceUsesOfWith(BB, NewBB);
+    //assert(!isa<CallBrInst>(Preds[i]->getTerminator()) &&
+           //"Cannot split an edge from a CallBrInst");
+    if (auto CBR = dyn_cast<CallBrInst>(T)) {
+      for (unsigned j = 0, E = CBR->getIndirectDests().size(); j != E; ++j)
+        if (CBR->getIndirectDest(j) == NewBB)
+          CBR->setIndirectDest(j, NewBB);
+    } else
+      T->replaceUsesOfWith(BB, NewBB);
+    llvm::errs() << "XXX: " << *T << "\n";
+    DT->print(llvm::errs());
+
   }
 
   // Insert a new PHI node into NewBB for every PHI node in BB and that new PHI
   // node becomes an incoming value for BB's phi node.  However, if the Preds
   // list is empty, we need to insert dummy entries into the PHI nodes in BB to
   // account for the newly created predecessor.
   if (Preds.empty()) {
     // Insert dummy values as the incoming value.

llvm/unittests/IR/InstructionsTest.cpp

TEST(InstructionsTest, FNegInstruction) {
   EXPECT_FALSE(FNeg->hasAllowReciprocal());
   EXPECT_FALSE(FNeg->hasAllowContract());
   EXPECT_FALSE(FNeg->hasAllowReassoc());
   EXPECT_FALSE(FNeg->hasApproxFunc());
   FAdd->deleteValue();
   FNeg->deleteValue();
 }
 
+void VerifyCallBr(const CallBrInst &CBI, const BasicBlock &Expected, const
+    std::string &F) {
+  BlockAddress *IndirectBA = BlockAddress::get(CBI.getIndirectDest(0));
+  BlockAddress *ArgBA = cast<BlockAddress>(CBI.getArgOperand(0));
+  EXPECT_EQ(IndirectBA, ArgBA)
+      << "After " << F << ", callbr had an indirect destination of '"
+      << CBI.getIndirectDest(0)->getName() << "', but a argument of '"
+      << ArgBA->getBasicBlock()->getName() << "'. These should always match:\n"
+      << CBI;
+  EXPECT_EQ(IndirectBA->getBasicBlock(), &Expected);
+  EXPECT_EQ(ArgBA->getBasicBlock(), &Expected);
+}
+
 TEST(InstructionsTest, CallBrInstruction) {
   LLVMContext Context;
   std::unique_ptr<Module> M = parseIR(Context, R"(
 define void @foo() {
 entry:
-  callbr void asm sideeffect "// XXX: ${0:l}", "X"(i8* blockaddress(@foo, %branch_test.exit))
+  callbr void asm sideeffect "", "X"(i8* blockaddress(@foo, %branch_test.exit))
           to label %land.rhs.i [label %branch_test.exit]
 
 land.rhs.i:
   br label %branch_test.exit
 
 branch_test.exit:
-  %0 = phi i1 [ true, %entry ], [ false, %land.rhs.i ]
-  br i1 %0, label %if.end, label %if.then
+  br label %if.then
 
 if.then:
   ret void
-
-if.end:
-  ret void
 }
 )");
   Function *Foo = M->getFunction("foo");
   auto BBs = Foo->getBasicBlockList().begin();
   CallBrInst &CBI = cast<CallBrInst>(BBs->front());
   ++BBs;
   ++BBs;
   BasicBlock &BranchTestExit = *BBs;
   ++BBs;
   BasicBlock &IfThen = *BBs;
 
   // Test that setting the first indirect destination of callbr updates the dest
   EXPECT_EQ(&BranchTestExit, CBI.getIndirectDest(0));
   CBI.setIndirectDest(0, &IfThen);
   EXPECT_EQ(&IfThen, CBI.getIndirectDest(0));
 
   // Further, test that changing the indirect destination updates the arg
   // operand to use the block address of the new indirect destination basic
   // block. This is a critical invariant of CallBrInst.
-  BlockAddress *IndirectBA = BlockAddress::get(CBI.getIndirectDest(0));
-  BlockAddress *ArgBA = cast<BlockAddress>(CBI.getArgOperand(0));
-  EXPECT_EQ(IndirectBA, ArgBA)
-      << "After setting the indirect destination, callbr had an indirect "
-         "destination of '"
-      << CBI.getIndirectDest(0)->getName() << "', but a argument of '"
-      << ArgBA->getBasicBlock()->getName() << "'. These should always match:\n"
-      << CBI;
-  EXPECT_EQ(IndirectBA->getBasicBlock(), &IfThen);
-  EXPECT_EQ(ArgBA->getBasicBlock(), &IfThen);
+  VerifyCallBr(CBI, IfThen, "CallBrInst::setIndirectDest()");
+
+  // Now try changing it back with User::replaceUsesOfWith().
+  CBI.replaceUsesOfWith(CBI.getIndirectDest(0), &BranchTestExit);
+  VerifyCallBr(CBI, BranchTestExit, "User::replaceUsesOfWith()");
 }
 
 TEST(InstructionsTest, UnaryOperator) {
   LLVMContext Context;
   IRBuilder<> Builder(Context);
   Instruction *I = Builder.CreatePHI(Builder.getDoubleTy(), 0);
   Value *F = Builder.CreateFNeg(I);