Daniel Sanders via llvm-dev
2016-Apr-22 10:35 UTC
[llvm-dev] [llvm] r267111 - [EarlyCSE] Take the intersection of flags on instructions
Hi David, When I reverted r267098 in r267127, one of the EarlyCSE tests failed on a couple builders (http://lab.llvm.org:8011/builders/llvm-hexagon-elf/builds/28174, http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/11173). Going by the logs it was CSE'ing two adds where one had the nuw flag and one didn't. The failures disappeared by themselves in the next build but I thought I should mention it anyway.> -----Original Message----- > From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf > Of David Majnemer via llvm-commits > Sent: 22 April 2016 07:38 > To: llvm-commits at lists.llvm.org > Subject: [llvm] r267111 - [EarlyCSE] Take the intersection of flags on > instructions > > 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=26711 > 1&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=26711 > 1&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=2671 > 10&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=aut > o > =========================================================> ===================> --- 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 > > > _______________________________________________ > llvm-commits mailing list > llvm-commits at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits