[llvm-dev] [llvm] r267111 - [EarlyCSE] Take the intersection of flags on instructions
Daniel Sanders via llvm-dev
llvm-dev at lists.llvm.org
Fri Apr 22 03:35:40 PDT 2016
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
More information about the llvm-dev
mailing list