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

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 22 07:06:31 PDT 2016


Thanks, taking a look.

On Friday, April 22, 2016, Mikael Holmén <mikael.holmen at ericsson.com> wrote:

> Same thing is happening to me.
>
>  Transforms/EarlyCSE/basic.ll
>
> is failing quite randomly now and then with:
>
> Command Output (stderr):
> --
> /repo/uabelho/dev-master/test/Transforms/EarlyCSE/basic.ll:31:16: error:
> expected string not found in input
>  ; CHECK-NEXT: %G = add nuw i32 %C, %C
>                ^
> <stdin>:14:23: note: scanning from here
>  store volatile i32 %E, i32* %P
>                       ^
> <stdin>:15:3: note: possible intended match here
>  store volatile i32 %E, i32* %P
>   ^
>
> Regards,
> Mikael
>
> On 04/22/2016 03:22 PM, Daniel Sanders via llvm-commits wrote:
>
>> 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
>>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160422/ab2963d7/attachment-0001.html>


More information about the llvm-commits mailing list