[llvm] r267111 - [EarlyCSE] Take the intersection of flags on instructions
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 22 07:40:59 PDT 2016
Thanks. I'll try to get the failing output if I see it again.
From: David Majnemer [mailto:david.majnemer at gmail.com]
Sent: 22 April 2016 15:20
To: Mikael Holmén
Cc: Daniel Sanders; llvm-commits at lists.llvm.org
Subject: Re: [llvm] r267111 - [EarlyCSE] Take the intersection of flags on instructions
Despite my best efforts, I couldn't reproduce this issue. However, inspection of the code yielded something suspicious and was fixed in r267153. Please let me know if you still have problems.
If you *do* have problems, please post a copy of the failing output of -early-cse so that I can figure out what is going wrong.
Thanks.
On Fri, Apr 22, 2016 at 10:06 AM, David Majnemer <david.majnemer at gmail.com<mailto:david.majnemer at gmail.com>> wrote:
Thanks, taking a look.
On Friday, April 22, 2016, Mikael Holmén <mikael.holmen at ericsson.com<mailto: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<mailto: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<mailto: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<mailto: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/c2ae21c0/attachment.html>
More information about the llvm-commits
mailing list