From 3cf415c6c367ced43175ebd1dc4bd9582c7f5376 Mon Sep 17 00:00:00 2001
From: "Duncan P. N. Exon Smith" <dexonsmith@apple.com>
Date: Wed, 23 Jun 2021 11:25:04 -0700
Subject: [PATCH] IR: Fix use-list-order round-tripping for call and invoke

Fix the use-list-order for call and invoke instructions by setting the
operands in order of their index. This matches the use-list-order
prediction. Note that the verifier precludes sharing operands in callbr
(so there was no bug to fix), but that code was updated for consistency.

Bug was found during review of https://reviews.llvm.org/D104740.

Differential Revision: https://reviews.llvm.org/D104805
---
 llvm/lib/IR/Instructions.cpp              | 24 ++++++++++++-------
 llvm/test/Assembler/call-arg-is-callee.ll | 39 +++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 9 deletions(-)
 create mode 100644 llvm/test/Assembler/call-arg-is-callee.ll

diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index eea4e81..73a5053 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -482,7 +482,6 @@ void CallInst::init(FunctionType *FTy, Value *Func, ArrayRef<Value *> Args,
   this->FTy = FTy;
   assert(getNumOperands() == Args.size() + CountBundleInputs(Bundles) + 1 &&
          "NumOperands not set up?");
-  setCalledOperand(Func);
 
 #ifndef NDEBUG
   assert((Args.size() == FTy->getNumParams() ||
@@ -495,7 +494,10 @@ void CallInst::init(FunctionType *FTy, Value *Func, ArrayRef<Value *> Args,
            "Calling a function with a bad signature!");
 #endif
 
+  // Set operands in order of their index to match use-list-order
+  // prediction.
   llvm::copy(Args, op_begin());
+  setCalledOperand(Func);
 
   auto It = populateBundleOperandInfos(Bundles, Args.size());
   (void)It;
@@ -824,9 +826,6 @@ void InvokeInst::init(FunctionType *FTy, Value *Fn, BasicBlock *IfNormal,
   assert((int)getNumOperands() ==
              ComputeNumOperands(Args.size(), CountBundleInputs(Bundles)) &&
          "NumOperands not set up?");
-  setNormalDest(IfNormal);
-  setUnwindDest(IfException);
-  setCalledOperand(Fn);
 
 #ifndef NDEBUG
   assert(((Args.size() == FTy->getNumParams()) ||
@@ -839,7 +838,12 @@ void InvokeInst::init(FunctionType *FTy, Value *Fn, BasicBlock *IfNormal,
            "Invoking a function with a bad signature!");
 #endif
 
+  // Set operands in order of their index to match use-list-order
+  // prediction.
   llvm::copy(Args, op_begin());
+  setNormalDest(IfNormal);
+  setUnwindDest(IfException);
+  setCalledOperand(Fn);
 
   auto It = populateBundleOperandInfos(Bundles, Args.size());
   (void)It;
@@ -892,11 +896,6 @@ void CallBrInst::init(FunctionType *FTy, Value *Fn, BasicBlock *Fallthrough,
              ComputeNumOperands(Args.size(), IndirectDests.size(),
                                 CountBundleInputs(Bundles)) &&
          "NumOperands not set up?");
-  NumIndirectDests = IndirectDests.size();
-  setDefaultDest(Fallthrough);
-  for (unsigned i = 0; i != NumIndirectDests; ++i)
-    setIndirectDest(i, IndirectDests[i]);
-  setCalledOperand(Fn);
 
 #ifndef NDEBUG
   assert(((Args.size() == FTy->getNumParams()) ||
@@ -909,7 +908,14 @@ void CallBrInst::init(FunctionType *FTy, Value *Fn, BasicBlock *Fallthrough,
            "Calling a function with a bad signature!");
 #endif
 
+  // Set operands in order of their index to match use-list-order
+  // prediction.
   std::copy(Args.begin(), Args.end(), op_begin());
+  NumIndirectDests = IndirectDests.size();
+  setDefaultDest(Fallthrough);
+  for (unsigned i = 0; i != NumIndirectDests; ++i)
+    setIndirectDest(i, IndirectDests[i]);
+  setCalledOperand(Fn);
 
   auto It = populateBundleOperandInfos(Bundles, Args.size());
   (void)It;
diff --git a/llvm/test/Assembler/call-arg-is-callee.ll b/llvm/test/Assembler/call-arg-is-callee.ll
new file mode 100644
index 0000000..42a9e4b
--- /dev/null
+++ b/llvm/test/Assembler/call-arg-is-callee.ll
@@ -0,0 +1,39 @@
+; RUN: llvm-as < %s -disable-output 2>&1 | FileCheck %s -allow-empty
+; CHECK-NOT: error
+; CHECK-NOT: warning
+; RUN: verify-uselistorder < %s
+
+; Check ordering of callee operand versus the argument operand.
+define void @call(void (...)* %p) {
+  call void (...) %p(void (...)* %p)
+  ret void
+}
+
+; Check ordering of callee operand versus the argument operand.
+declare void @personality(i8*)
+define void @invoke(void (...)* %p) personality void(i8*)* @personality {
+entry:
+  invoke void (...) %p(void (...)* %p)
+  to label %normal unwind label %exception
+normal:
+  ret void
+exception:
+  landingpad { i8*, i32 } cleanup
+  ret void
+}
+
+; Check order for callbr instruction. Cannot reuse labels in the test since the
+; verifier prevents duplicating callbr destinations.
+define void @callbr() {
+entry:
+  callbr i32 asm "", "=r,r,X,X"(i32 0,
+                                i8 *blockaddress(@callbr, %two),
+                                i8 *blockaddress(@callbr, %three))
+              to label %one [label %two, label %three]
+one:
+  ret void
+two:
+  ret void
+three:
+  ret void
+}
-- 
2.7.4