[llvm] r267111 - [EarlyCSE] Take the intersection of flags on instructions

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 21 23:37:46 PDT 2016


Author: majnemer
Date: Fri Apr 22 01:37:45 2016
New Revision: 267111

URL: http://llvm.org/viewvc/llvm-project?rev=267111&view=rev
Log:
[EarlyCSE] Take the intersection of flags on instructions

EarlyCSE had inconsistent behavior with regards to flag'd instructions:
- In some cases, it would pessimize if the available instruction had
  different flags by not performing CSE.
- In other cases, it would miscompile if it replaced an instruction
  which had no flags with an instruction which has flags.

Fix this by being more consistent with our flag handling by utilizing
andIRFlags.

Added:
    llvm/trunk/test/Transforms/EarlyCSE/flags.ll
Modified:
    llvm/trunk/include/llvm/IR/InstrTypes.h
    llvm/trunk/include/llvm/IR/Instruction.h
    llvm/trunk/include/llvm/IR/Operator.h
    llvm/trunk/lib/IR/Instruction.cpp
    llvm/trunk/lib/IR/Instructions.cpp
    llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp

Modified: llvm/trunk/include/llvm/IR/InstrTypes.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/InstrTypes.h?rev=267111&r1=267110&r2=267111&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/InstrTypes.h (original)
+++ llvm/trunk/include/llvm/IR/InstrTypes.h Fri Apr 22 01:37:45 2016
@@ -535,35 +535,6 @@ public:
   ///
   bool swapOperands();
 
-  /// Set or clear the nsw flag on this instruction, which must be an operator
-  /// which supports this flag. See LangRef.html for the meaning of this flag.
-  void setHasNoUnsignedWrap(bool b = true);
-
-  /// Set or clear the nsw flag on this instruction, which must be an operator
-  /// which supports this flag. See LangRef.html for the meaning of this flag.
-  void setHasNoSignedWrap(bool b = true);
-
-  /// Set or clear the exact flag on this instruction, which must be an operator
-  /// which supports this flag. See LangRef.html for the meaning of this flag.
-  void setIsExact(bool b = true);
-
-  /// Determine whether the no unsigned wrap flag is set.
-  bool hasNoUnsignedWrap() const;
-
-  /// Determine whether the no signed wrap flag is set.
-  bool hasNoSignedWrap() const;
-
-  /// Determine whether the exact flag is set.
-  bool isExact() const;
-
-  /// Convenience method to copy supported wrapping, exact, and fast-math flags
-  /// from V to this instruction.
-  void copyIRFlags(const Value *V);
-
-  /// Logical 'and' of any supported wrapping, exact, and fast-math flags of
-  /// V and this instruction.
-  void andIRFlags(const Value *V);
-
   // Methods for support type inquiry through isa, cast, and dyn_cast:
   static inline bool classof(const Instruction *I) {
     return I->isBinaryOp();

Modified: llvm/trunk/include/llvm/IR/Instruction.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Instruction.h?rev=267111&r1=267110&r2=267111&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Instruction.h (original)
+++ llvm/trunk/include/llvm/IR/Instruction.h Fri Apr 22 01:37:45 2016
@@ -223,6 +223,27 @@ public:
   /// Return the debug location for this node as a DebugLoc.
   const DebugLoc &getDebugLoc() const { return DbgLoc; }
 
+  /// Set or clear the nsw flag on this instruction, which must be an operator
+  /// which supports this flag. See LangRef.html for the meaning of this flag.
+  void setHasNoUnsignedWrap(bool b = true);
+
+  /// Set or clear the nsw flag on this instruction, which must be an operator
+  /// which supports this flag. See LangRef.html for the meaning of this flag.
+  void setHasNoSignedWrap(bool b = true);
+
+  /// Set or clear the exact flag on this instruction, which must be an operator
+  /// which supports this flag. See LangRef.html for the meaning of this flag.
+  void setIsExact(bool b = true);
+
+  /// Determine whether the no unsigned wrap flag is set.
+  bool hasNoUnsignedWrap() const;
+
+  /// Determine whether the no signed wrap flag is set.
+  bool hasNoSignedWrap() const;
+
+  /// Determine whether the exact flag is set.
+  bool isExact() const;
+
   /// Set or clear the unsafe-algebra flag on this instruction, which must be an
   /// operator which supports this flag. See LangRef.html for the meaning of
   /// this flag.
@@ -281,6 +302,14 @@ public:
   /// Copy I's fast-math flags
   void copyFastMathFlags(const Instruction *I);
 
+  /// Convenience method to copy supported wrapping, exact, and fast-math flags
+  /// from V to this instruction.
+  void copyIRFlags(const Value *V);
+
+  /// Logical 'and' of any supported wrapping, exact, and fast-math flags of
+  /// V and this instruction.
+  void andIRFlags(const Value *V);
+
 private:
   /// Return true if we have an entry in the on-the-side metadata hash.
   bool hasMetadataHashEntry() const {

Modified: llvm/trunk/include/llvm/IR/Operator.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Operator.h?rev=267111&r1=267110&r2=267111&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Operator.h (original)
+++ llvm/trunk/include/llvm/IR/Operator.h Fri Apr 22 01:37:45 2016
@@ -79,7 +79,7 @@ public:
   };
 
 private:
-  friend class BinaryOperator;
+  friend class Instruction;
   friend class ConstantExpr;
   void setHasNoUnsignedWrap(bool B) {
     SubclassOptionalData =
@@ -130,7 +130,7 @@ public:
   };
 
 private:
-  friend class BinaryOperator;
+  friend class Instruction;
   friend class ConstantExpr;
   void setIsExact(bool B) {
     SubclassOptionalData = (SubclassOptionalData & ~IsExact) | (B * IsExact);

Modified: llvm/trunk/lib/IR/Instruction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Instruction.cpp?rev=267111&r1=267110&r2=267111&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Instruction.cpp (original)
+++ llvm/trunk/lib/IR/Instruction.cpp Fri Apr 22 01:37:45 2016
@@ -96,6 +96,30 @@ void Instruction::moveBefore(Instruction
       MovePos->getIterator(), getParent()->getInstList(), getIterator());
 }
 
+void Instruction::setHasNoUnsignedWrap(bool b) {
+  cast<OverflowingBinaryOperator>(this)->setHasNoUnsignedWrap(b);
+}
+
+void Instruction::setHasNoSignedWrap(bool b) {
+  cast<OverflowingBinaryOperator>(this)->setHasNoSignedWrap(b);
+}
+
+void Instruction::setIsExact(bool b) {
+  cast<PossiblyExactOperator>(this)->setIsExact(b);
+}
+
+bool Instruction::hasNoUnsignedWrap() const {
+  return cast<OverflowingBinaryOperator>(this)->hasNoUnsignedWrap();
+}
+
+bool Instruction::hasNoSignedWrap() const {
+  return cast<OverflowingBinaryOperator>(this)->hasNoSignedWrap();
+}
+
+bool Instruction::isExact() const {
+  return cast<PossiblyExactOperator>(this)->isExact();
+}
+
 /// Set or clear the unsafe-algebra flag on this instruction, which must be an
 /// operator which supports this flag. See LangRef.html for the meaning of this
 /// flag.
@@ -190,6 +214,37 @@ void Instruction::copyFastMathFlags(cons
   copyFastMathFlags(I->getFastMathFlags());
 }
 
+void Instruction::copyIRFlags(const Value *V) {
+  // Copy the wrapping flags.
+  if (auto *OB = dyn_cast<OverflowingBinaryOperator>(V)) {
+    setHasNoSignedWrap(OB->hasNoSignedWrap());
+    setHasNoUnsignedWrap(OB->hasNoUnsignedWrap());
+  }
+
+  // Copy the exact flag.
+  if (auto *PE = dyn_cast<PossiblyExactOperator>(V))
+    setIsExact(PE->isExact());
+
+  // Copy the fast-math flags.
+  if (auto *FP = dyn_cast<FPMathOperator>(V))
+    copyFastMathFlags(FP->getFastMathFlags());
+}
+
+void Instruction::andIRFlags(const Value *V) {
+  if (auto *OB = dyn_cast<OverflowingBinaryOperator>(V)) {
+    setHasNoSignedWrap(hasNoSignedWrap() & OB->hasNoSignedWrap());
+    setHasNoUnsignedWrap(hasNoUnsignedWrap() & OB->hasNoUnsignedWrap());
+  }
+
+  if (auto *PE = dyn_cast<PossiblyExactOperator>(V))
+    setIsExact(isExact() & PE->isExact());
+
+  if (auto *FP = dyn_cast<FPMathOperator>(V)) {
+    FastMathFlags FM = getFastMathFlags();
+    FM &= FP->getFastMathFlags();
+    copyFastMathFlags(FM);
+  }
+}
 
 const char *Instruction::getOpcodeName(unsigned OpCode) {
   switch (OpCode) {

Modified: llvm/trunk/lib/IR/Instructions.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Instructions.cpp?rev=267111&r1=267110&r2=267111&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Instructions.cpp (original)
+++ llvm/trunk/lib/IR/Instructions.cpp Fri Apr 22 01:37:45 2016
@@ -2201,62 +2201,6 @@ bool BinaryOperator::swapOperands() {
   return false;
 }
 
-void BinaryOperator::setHasNoUnsignedWrap(bool b) {
-  cast<OverflowingBinaryOperator>(this)->setHasNoUnsignedWrap(b);
-}
-
-void BinaryOperator::setHasNoSignedWrap(bool b) {
-  cast<OverflowingBinaryOperator>(this)->setHasNoSignedWrap(b);
-}
-
-void BinaryOperator::setIsExact(bool b) {
-  cast<PossiblyExactOperator>(this)->setIsExact(b);
-}
-
-bool BinaryOperator::hasNoUnsignedWrap() const {
-  return cast<OverflowingBinaryOperator>(this)->hasNoUnsignedWrap();
-}
-
-bool BinaryOperator::hasNoSignedWrap() const {
-  return cast<OverflowingBinaryOperator>(this)->hasNoSignedWrap();
-}
-
-bool BinaryOperator::isExact() const {
-  return cast<PossiblyExactOperator>(this)->isExact();
-}
-
-void BinaryOperator::copyIRFlags(const Value *V) {
-  // Copy the wrapping flags.
-  if (auto *OB = dyn_cast<OverflowingBinaryOperator>(V)) {
-    setHasNoSignedWrap(OB->hasNoSignedWrap());
-    setHasNoUnsignedWrap(OB->hasNoUnsignedWrap());
-  }
-
-  // Copy the exact flag.
-  if (auto *PE = dyn_cast<PossiblyExactOperator>(V))
-    setIsExact(PE->isExact());
-  
-  // Copy the fast-math flags.
-  if (auto *FP = dyn_cast<FPMathOperator>(V))
-    copyFastMathFlags(FP->getFastMathFlags());
-}
-
-void BinaryOperator::andIRFlags(const Value *V) {
-  if (auto *OB = dyn_cast<OverflowingBinaryOperator>(V)) {
-    setHasNoSignedWrap(hasNoSignedWrap() & OB->hasNoSignedWrap());
-    setHasNoUnsignedWrap(hasNoUnsignedWrap() & OB->hasNoUnsignedWrap());
-  }
-  
-  if (auto *PE = dyn_cast<PossiblyExactOperator>(V))
-    setIsExact(isExact() & PE->isExact());
-  
-  if (auto *FP = dyn_cast<FPMathOperator>(V)) {
-    FastMathFlags FM = getFastMathFlags();
-    FM &= FP->getFastMathFlags();
-    copyFastMathFlags(FM);
-  }
-}
-
 
 //===----------------------------------------------------------------------===//
 //                             FPMathOperator Class

Modified: llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp?rev=267111&r1=267110&r2=267111&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp Fri Apr 22 01:37:45 2016
@@ -153,7 +153,7 @@ bool DenseMapInfo<SimpleValue>::isEqual(
 
   if (LHSI->getOpcode() != RHSI->getOpcode())
     return false;
-  if (LHSI->isIdenticalTo(RHSI))
+  if (LHSI->isIdenticalToWhenDefined(RHSI))
     return true;
 
   // If we're not strictly identical, we still might be a commutable instruction
@@ -165,15 +165,6 @@ bool DenseMapInfo<SimpleValue>::isEqual(
            "same opcode, but different instruction type?");
     BinaryOperator *RHSBinOp = cast<BinaryOperator>(RHSI);
 
-    // Check overflow attributes
-    if (isa<OverflowingBinaryOperator>(LHSBinOp)) {
-      assert(isa<OverflowingBinaryOperator>(RHSBinOp) &&
-             "same opcode, but different operator type?");
-      if (LHSBinOp->hasNoUnsignedWrap() != RHSBinOp->hasNoUnsignedWrap() ||
-          LHSBinOp->hasNoSignedWrap() != RHSBinOp->hasNoSignedWrap())
-        return false;
-    }
-
     // Commuted equality
     return LHSBinOp->getOperand(0) == RHSBinOp->getOperand(1) &&
            LHSBinOp->getOperand(1) == RHSBinOp->getOperand(0);
@@ -584,6 +575,8 @@ bool EarlyCSE::processNode(DomTreeNode *
       // See if the instruction has an available value.  If so, use it.
       if (Value *V = AvailableValues.lookup(Inst)) {
         DEBUG(dbgs() << "EarlyCSE CSE: " << *Inst << "  to: " << *V << '\n');
+        if (auto *I = dyn_cast<Instruction>(V))
+          I->andIRFlags(Inst);
         Inst->replaceAllUsesWith(V);
         Inst->eraseFromParent();
         Changed = true;

Added: llvm/trunk/test/Transforms/EarlyCSE/flags.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/EarlyCSE/flags.ll?rev=267111&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/EarlyCSE/flags.ll (added)
+++ llvm/trunk/test/Transforms/EarlyCSE/flags.ll Fri Apr 22 01:37:45 2016
@@ -0,0 +1,18 @@
+; RUN: opt -early-cse -S < %s | FileCheck %s
+
+declare void @use(i1)
+
+define void @test1(float %x, float %y) {
+entry:
+  %cmp1 = fcmp nnan oeq float %y, %x
+  %cmp2 = fcmp oeq float %x, %y
+  call void @use(i1 %cmp1)
+  call void @use(i1 %cmp2)
+  ret void
+}
+
+; CHECK-LABEL: define void @test1(
+; CHECK: %[[cmp:.*]] = fcmp oeq float %y, %x
+; CHECK-NEXT: call void @use(i1 %[[cmp]])
+; CHECK-NEXT: call void @use(i1 %[[cmp]])
+; CHECK-NEXT: ret void




More information about the llvm-commits mailing list