Added hasSamePropertiesAs method for CmpXchgInst and FenceInst

[vish99]

Summary

Added hasSamePropertiesAs() method for CmpXchgInst and FenceInst, which checks for instruction specific properties of both the instructions and changed FunctionComparator to use these functions in the cmpOperations() method.

[jfb]

llvm/include/llvm/IR/Instruction.h

682

We have isIdenticalTo above, and then we have isSameOperationAs. The later details how it's different from the former. You ought to do the same thing here.

llvm/lib/IR/Instructions.cpp

1567

Can you compare ordering directly instead of casting, here and below?

llvm/include/llvm/IR/Instruction.h

public:
   /// types and any other factors affecting the operation must be the same. This
   /// is similar to isIdenticalTo except the operands themselves don't have to
   /// be identical.
   /// @returns true if the specified instruction is the same operation as
   /// the current one.
   /// Determine if one instruction is the same operation as another.
   bool isSameOperationAs(const Instruction *I, unsigned flags = 0) const;
 
+  // This function checks if operands of two instructions are same.
+  // The Opcode and number of operands for both the instructions need
+  // to be the same. This method is made to assert false to avoid
+  // comparisions between instructions whose subclass is not known.
+  int hasSamePropertiesAs(const Instruction *I) const;
+
   /// Return true if there are any uses of this instruction in blocks other than
   /// the specified block. Note that PHI nodes are considered to evaluate their
   /// operands in the corresponding predecessor block.
   bool isUsedOutsideOfBlock(const BasicBlock *BB) const;
 
   /// Return the number of successors that this instruction has. The instruction
   /// must be a terminator.
   unsigned getNumSuccessors() const;

llvm/include/llvm/IR/Instructions.h

public:
   // Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const Instruction *I) {
     return I->getOpcode() == Instruction::Fence;
   }
   static bool classof(const Value *V) {
     return isa<Instruction>(V) && classof(cast<Instruction>(V));
   }
 
+  // This function checks if operands of two FenceInsts are same.
+  // The Opcode and number of operands for both the instructions need to
+  // be the same
+  int hasSamePropertiesAs(const Instruction *I) const;
+
 private:
   // Shadow Instruction::setInstructionSubclassData with a private forwarding
   // method so that subclasses cannot accidentally use it.
   void setInstructionSubclassData(unsigned short D) {
     Instruction::setInstructionSubclassData(D);
   }
 
   /// The synchronization scope ID of this fence instruction.  Not quite enough
public:
   // Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const Instruction *I) {
     return I->getOpcode() == Instruction::AtomicCmpXchg;
   }
   static bool classof(const Value *V) {
     return isa<Instruction>(V) && classof(cast<Instruction>(V));
   }
 
+  // This function checks if operands of two AtomicCmpXchgInsts are same.
+  // The Opcode and number of operands for both the instructions need to
+  // be the same
+  int hasSamePropertiesAs(const Instruction *I) const;
+
 private:
   // Shadow Instruction::setInstructionSubclassData with a private forwarding
   // method so that subclasses cannot accidentally use it.
   void setInstructionSubclassData(unsigned short D) {
     Instruction::setInstructionSubclassData(D);
   }
 
   /// The synchronization scope ID of this cmpxchg instruction.  Not quite

llvm/lib/IR/Instruction.cpp

if (UseScalarTypes ?
         getOperand(i)->getType()->getScalarType() !=
           I->getOperand(i)->getType()->getScalarType() :
         getOperand(i)->getType() != I->getOperand(i)->getType())
       return false;
 
   return haveSameSpecialState(this, I, IgnoreAlignment);
 }
 
+int Instruction::hasSamePropertiesAs(const Instruction *I) const {
+  assert(
+      false &&
+      "Cannot compare properties of instructions whose subclass is not known");
+  llvm_unreachable("Properties to compare are not known");
+}
+
 bool Instruction::isUsedOutsideOfBlock(const BasicBlock *BB) const {
   for (const Use &U : uses()) {
     // PHI nodes uses values in the corresponding predecessor block.  For other
     // instructions, just check to see whether the parent of the use matches up.
     const Instruction *I = cast<Instruction>(U.getUser());
     const PHINode *PN = dyn_cast<PHINode>(I);
     if (!PN) {
       if (I->getParent() != BB)

llvm/lib/IR/Instructions.cpp

 #include "llvm/Support/TypeSize.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
 #include <vector>
 
 using namespace llvm;
 
+int cmpIntegers(uint64_t L, uint64_t R) {
+  if (L > R)
+    return 1;
+  if (L < R)
+    return -1;
+  return 0;
+}
+
 //===----------------------------------------------------------------------===//
 //                            AllocaInst Class
 //===----------------------------------------------------------------------===//
 
 Optional<uint64_t>
 AllocaInst::getAllocationSizeInBits(const DataLayout &DL) const {
   uint64_t Size = DL.getTypeAllocSizeInBits(getAllocatedType());
   if (isArrayAllocation()) {
AtomicCmpXchgInst::AtomicCmpXchgInst(Value *Ptr, Value *Cmp, Value *NewVal,
                                      BasicBlock *InsertAtEnd)
     : Instruction(
           StructType::get(Cmp->getType(), Type::getInt1Ty(Cmp->getContext())),
           AtomicCmpXchg, OperandTraits<AtomicCmpXchgInst>::op_begin(this),
           OperandTraits<AtomicCmpXchgInst>::operands(this), InsertAtEnd) {
   Init(Ptr, Cmp, NewVal, SuccessOrdering, FailureOrdering, SSID);
 }
 
+int AtomicCmpXchgInst::hasSamePropertiesAs(const Instruction *I) const {
+  assert(getOpcode() == I->getOpcode() &&
+         "Cannot check properties of instructions with different Opcodes");
+  assert(getNumOperands() = I->getNumOperands() &&
+                            "Cannot check properties of instructions "
+                            "with different number of operands");
+  if (int Res = cmpIntegers(this->isVolatile(),
+                            cast<AtomicCmpXchgInst>(I)->isVolatile()))
+    return Res;
+  if (int Res =
+          cmpIntegers(this->isWeak(), cast<AtomicCmpXchgInst>(I)->isWeak()))
+    return Res;
+  if (int Res =
+          cmpIntegers(int(this->getSuccessOrdering()),
+                      int(cast<AtomicCmpXchgInst>(I)->getSuccessOrdering())))
+    return Res;
+  if (int Res =
+          cmpIntegers(int(this->getFailureOrdering()),
+                      int(cast<AtomicCmpXchgInst>(I)->getFailureOrdering())))
+    return Res;
+  return cmpIntegers(this->getSyncScopeID(),
+                     cast<AtomicCmpXchgInst>(I)->getSyncScopeID());
+}
+
 //===----------------------------------------------------------------------===//
 //                       AtomicRMWInst Implementation
 //===----------------------------------------------------------------------===//
 
 void AtomicRMWInst::Init(BinOp Operation, Value *Ptr, Value *Val,
                          AtomicOrdering Ordering,
                          SyncScope::ID SSID) {
   Op<0>() = Ptr;
FenceInst::FenceInst(LLVMContext &C, AtomicOrdering Ordering,
                      SyncScope::ID SSID,
                      Instruction *InsertBefore)
   : Instruction(Type::getVoidTy(C), Fence, nullptr, 0, InsertBefore) {
   setOrdering(Ordering);
   setSyncScopeID(SSID);
 }
 
 FenceInst::FenceInst(LLVMContext &C, AtomicOrdering Ordering,
-                     SyncScope::ID SSID,
-                     BasicBlock *InsertAtEnd)
+                     SyncScope::ID SSID, BasicBlock *InsertAtEnd)
     : Instruction(Type::getVoidTy(C), Fence, nullptr, 0, InsertAtEnd) {
   setOrdering(Ordering);
   setSyncScopeID(SSID);
 }
 
+int FenceInst::hasSamePropertiesAs(const Instruction *I) const {
+  assert(getOpcode() == I->getOpcode() &&
+         "Cannot check properties of instructions with different Opcodes");
+  assert(getNumOperands() = I->getNumOperands() &&
+                            "Cannot check properties of instructions "
+                            "with different number of operands");
+  if (int Res = cmpIntegers(int(this->getOrdering()),
+                            int(cast<FenceInst>(I)->getOrdering())))
+    return Res;
+  return cmpIntegers(this->getSyncScopeID(),
+                     cast<FenceInst>(I)->getSyncScopeID());
+}
+
 //===----------------------------------------------------------------------===//
 //                       GetElementPtrInst Implementation
 //===----------------------------------------------------------------------===//
 
 void GetElementPtrInst::init(Value *Ptr, ArrayRef<Value *> IdxList,
                              const Twine &Name) {
   assert(getNumOperands() == 1 + IdxList.size() &&
          "NumOperands not initialized?");

llvm/lib/Transforms/Utils/FunctionComparator.cpp

if (const ExtractValueInst *EVI = dyn_cast<ExtractValueInst>(L)) {
     if (int Res = cmpNumbers(LIndices.size(), RIndices.size()))
       return Res;
     for (size_t i = 0, e = LIndices.size(); i != e; ++i) {
       if (int Res = cmpNumbers(LIndices[i], RIndices[i]))
         return Res;
     }
   }
   if (const FenceInst *FI = dyn_cast<FenceInst>(L)) {
-    if (int Res =
-            cmpOrderings(FI->getOrdering(), cast<FenceInst>(R)->getOrdering()))
-      return Res;
-    return cmpNumbers(FI->getSyncScopeID(),
-                      cast<FenceInst>(R)->getSyncScopeID());
+    return FI->hasSamePropertiesAs(R);
   }
   if (const AtomicCmpXchgInst *CXI = dyn_cast<AtomicCmpXchgInst>(L)) {
-    if (int Res = cmpNumbers(CXI->isVolatile(),
-                             cast<AtomicCmpXchgInst>(R)->isVolatile()))
-      return Res;
-    if (int Res =
-            cmpNumbers(CXI->isWeak(), cast<AtomicCmpXchgInst>(R)->isWeak()))
-      return Res;
-    if (int Res =
-            cmpOrderings(CXI->getSuccessOrdering(),
-                         cast<AtomicCmpXchgInst>(R)->getSuccessOrdering()))
-      return Res;
-    if (int Res =
-            cmpOrderings(CXI->getFailureOrdering(),
-                         cast<AtomicCmpXchgInst>(R)->getFailureOrdering()))
-      return Res;
-    return cmpNumbers(CXI->getSyncScopeID(),
-                      cast<AtomicCmpXchgInst>(R)->getSyncScopeID());
+    return CXI->hasSamePropertiesAs(R);
   }
   if (const AtomicRMWInst *RMWI = dyn_cast<AtomicRMWInst>(L)) {
     if (int Res = cmpNumbers(RMWI->getOperation(),
                              cast<AtomicRMWInst>(R)->getOperation()))
       return Res;
     if (int Res = cmpNumbers(RMWI->isVolatile(),
                              cast<AtomicRMWInst>(R)->isVolatile()))
       return Res;