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

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 22 06:22:28 PDT 2016


Resent to the correct mailing list. Outlook makes me re-add the list on every reply and I didn't notice I'd autocompleted to the wrong one.

> -----Original Message-----
> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of Daniel
> Sanders via llvm-dev
> Sent: 22 April 2016 11:36
> To: David Majnemer; llvm-dev at lists.llvm.org
> Subject: Re: [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
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev


More information about the llvm-commits mailing list