[llvm-commits] [llvm] r105540 - in /llvm/trunk: include/llvm/Analysis/ScalarEvolution.h lib/Analysis/ScalarEvolution.cpp test/Analysis/ScalarEvolution/2008-07-29-SMinExpr.ll
Daniel Dunbar
daniel at zuster.org
Tue Jun 8 08:09:35 PDT 2010
Hi Dan,
This broke '2008-07-29-sminexpr.ll' when using Clang to compile, or
when using the system compiler on Darwin/PowerPC. I believe that
something is non-deterministic with your change, because the diff
shows LHS/RHS flipped from the expected output:
--
/Users/buildslave/zorg/buildbot/smooshlab/slave/build.clang-x86_64-darwin10-selfhost-rel/llvm.src/test/Analysis/ScalarEvolution/2008-07-29-SMinExpr.ll:25:10:
error: expected string not found in input
; CHECK: backedge-taken count is (-2147483632 + ((-1 + (-1 * %y)) smax
(-1 + (-1 * %x))))
^
<stdin>:1:1: note: scanning from here
Printing analysis 'Scalar Evolution Analysis' for function 'b':
^
<stdin>:14:15: note: possible intended match here
Loop %forinc: backedge-taken count is (-2147483632 + ((-1 + (-1 * %x))
smax (-1 + (-1 * %y))))
^
--
Can you take a look?
- Daniel
On Mon, Jun 7, 2010 at 12:06 PM, Dan Gohman <gohman at apple.com> wrote:
> Author: djg
> Date: Mon Jun 7 14:06:13 2010
> New Revision: 105540
>
> URL: http://llvm.org/viewvc/llvm-project?rev=105540&view=rev
> Log:
> Optimize ScalarEvolution's SCEVComplexityCompare predicate: don't go
> scrounging through SCEVUnknown contents and SCEVNAryExpr operands;
> instead just do a simple deterministic comparison of the precomputed
> hash data.
>
> Also, since this is more precise, it eliminates the need for the slow
> N^2 duplicate detection code.
>
> Modified:
> llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
> llvm/trunk/lib/Analysis/ScalarEvolution.cpp
> llvm/trunk/test/Analysis/ScalarEvolution/2008-07-29-SMinExpr.ll
>
> Modified: llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ScalarEvolution.h?rev=105540&r1=105539&r2=105540&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Analysis/ScalarEvolution.h (original)
> +++ llvm/trunk/include/llvm/Analysis/ScalarEvolution.h Mon Jun 7 14:06:13 2010
> @@ -76,6 +76,9 @@
> /// Profile - FoldingSet support.
> void Profile(FoldingSetNodeID& ID) { ID = FastID; }
>
> + /// getProfile - Like Profile, but a different interface which doesn't copy.
> + const FoldingSetNodeIDRef &getProfile() const { return FastID; }
> +
> /// isLoopInvariant - Return true if the value of this SCEV is unchanging in
> /// the specified loop.
> virtual bool isLoopInvariant(const Loop *L) const = 0;
>
> Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=105540&r1=105539&r2=105540&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
> +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Mon Jun 7 14:06:13 2010
> @@ -508,106 +508,15 @@
> if (LHS->getSCEVType() != RHS->getSCEVType())
> return LHS->getSCEVType() < RHS->getSCEVType();
>
> - // Aside from the getSCEVType() ordering, the particular ordering
> - // isn't very important except that it's beneficial to be consistent,
> - // so that (a + b) and (b + a) don't end up as different expressions.
> -
> - // Sort SCEVUnknown values with some loose heuristics. TODO: This is
> - // not as complete as it could be.
> - if (const SCEVUnknown *LU = dyn_cast<SCEVUnknown>(LHS)) {
> - const SCEVUnknown *RU = cast<SCEVUnknown>(RHS);
> -
> - // Order pointer values after integer values. This helps SCEVExpander
> - // form GEPs.
> - if (LU->getType()->isPointerTy() && !RU->getType()->isPointerTy())
> - return false;
> - if (RU->getType()->isPointerTy() && !LU->getType()->isPointerTy())
> - return true;
> -
> - // Compare getValueID values.
> - if (LU->getValue()->getValueID() != RU->getValue()->getValueID())
> - return LU->getValue()->getValueID() < RU->getValue()->getValueID();
> -
> - // Sort arguments by their position.
> - if (const Argument *LA = dyn_cast<Argument>(LU->getValue())) {
> - const Argument *RA = cast<Argument>(RU->getValue());
> - return LA->getArgNo() < RA->getArgNo();
> - }
> -
> - // For instructions, compare their loop depth, and their opcode.
> - // This is pretty loose.
> - if (Instruction *LV = dyn_cast<Instruction>(LU->getValue())) {
> - Instruction *RV = cast<Instruction>(RU->getValue());
> -
> - // Compare loop depths.
> - if (LI->getLoopDepth(LV->getParent()) !=
> - LI->getLoopDepth(RV->getParent()))
> - return LI->getLoopDepth(LV->getParent()) <
> - LI->getLoopDepth(RV->getParent());
> -
> - // Compare opcodes.
> - if (LV->getOpcode() != RV->getOpcode())
> - return LV->getOpcode() < RV->getOpcode();
> -
> - // Compare the number of operands.
> - if (LV->getNumOperands() != RV->getNumOperands())
> - return LV->getNumOperands() < RV->getNumOperands();
> - }
> -
> - return false;
> - }
> -
> - // Compare constant values.
> - if (const SCEVConstant *LC = dyn_cast<SCEVConstant>(LHS)) {
> - const SCEVConstant *RC = cast<SCEVConstant>(RHS);
> - if (LC->getValue()->getBitWidth() != RC->getValue()->getBitWidth())
> - return LC->getValue()->getBitWidth() < RC->getValue()->getBitWidth();
> - return LC->getValue()->getValue().ult(RC->getValue()->getValue());
> - }
> -
> - // Compare addrec loop depths.
> - if (const SCEVAddRecExpr *LA = dyn_cast<SCEVAddRecExpr>(LHS)) {
> - const SCEVAddRecExpr *RA = cast<SCEVAddRecExpr>(RHS);
> - if (LA->getLoop()->getLoopDepth() != RA->getLoop()->getLoopDepth())
> - return LA->getLoop()->getLoopDepth() < RA->getLoop()->getLoopDepth();
> - }
> -
> - // Lexicographically compare n-ary expressions.
> - if (const SCEVNAryExpr *LC = dyn_cast<SCEVNAryExpr>(LHS)) {
> - const SCEVNAryExpr *RC = cast<SCEVNAryExpr>(RHS);
> - for (unsigned i = 0, e = LC->getNumOperands(); i != e; ++i) {
> - if (i >= RC->getNumOperands())
> - return false;
> - if (operator()(LC->getOperand(i), RC->getOperand(i)))
> - return true;
> - if (operator()(RC->getOperand(i), LC->getOperand(i)))
> - return false;
> - }
> - return LC->getNumOperands() < RC->getNumOperands();
> - }
> -
> - // Lexicographically compare udiv expressions.
> - if (const SCEVUDivExpr *LC = dyn_cast<SCEVUDivExpr>(LHS)) {
> - const SCEVUDivExpr *RC = cast<SCEVUDivExpr>(RHS);
> - if (operator()(LC->getLHS(), RC->getLHS()))
> - return true;
> - if (operator()(RC->getLHS(), LC->getLHS()))
> - return false;
> - if (operator()(LC->getRHS(), RC->getRHS()))
> - return true;
> - if (operator()(RC->getRHS(), LC->getRHS()))
> - return false;
> - return false;
> - }
> -
> - // Compare cast expressions by operand.
> - if (const SCEVCastExpr *LC = dyn_cast<SCEVCastExpr>(LHS)) {
> - const SCEVCastExpr *RC = cast<SCEVCastExpr>(RHS);
> - return operator()(LC->getOperand(), RC->getOperand());
> - }
> -
> - llvm_unreachable("Unknown SCEV kind!");
> - return false;
> + // Then, pick an arbitrary sort. Use the profiling data for speed.
> + const FoldingSetNodeIDRef &L = LHS->getProfile();
> + const FoldingSetNodeIDRef &R = RHS->getProfile();
> + size_t LSize = L.getSize();
> + size_t RSize = R.getSize();
> + if (LSize != RSize)
> + return LSize < RSize;
> + return memcmp(L.getData(), R.getData(),
> + LSize * sizeof(*L.getData())) < 0;
> }
> };
> }
> @@ -625,36 +534,18 @@
> static void GroupByComplexity(SmallVectorImpl<const SCEV *> &Ops,
> LoopInfo *LI) {
> if (Ops.size() < 2) return; // Noop
> +
> + SCEVComplexityCompare Comp(LI);
> +
> if (Ops.size() == 2) {
> // This is the common case, which also happens to be trivially simple.
> // Special case it.
> - if (SCEVComplexityCompare(LI)(Ops[1], Ops[0]))
> + if (Comp(Ops[1], Ops[0]))
> std::swap(Ops[0], Ops[1]);
> return;
> }
>
> - // Do the rough sort by complexity.
> - std::stable_sort(Ops.begin(), Ops.end(), SCEVComplexityCompare(LI));
> -
> - // Now that we are sorted by complexity, group elements of the same
> - // complexity. Note that this is, at worst, N^2, but the vector is likely to
> - // be extremely short in practice. Note that we take this approach because we
> - // do not want to depend on the addresses of the objects we are grouping.
> - for (unsigned i = 0, e = Ops.size(); i != e-2; ++i) {
> - const SCEV *S = Ops[i];
> - unsigned Complexity = S->getSCEVType();
> -
> - // If there are any objects of the same complexity and same value as this
> - // one, group them.
> - for (unsigned j = i+1; j != e && Ops[j]->getSCEVType() == Complexity; ++j) {
> - if (Ops[j] == S) { // Found a duplicate.
> - // Move it to immediately after i'th element.
> - std::swap(Ops[i+1], Ops[j]);
> - ++i; // no need to rescan it.
> - if (i == e-2) return; // Done!
> - }
> - }
> - }
> + std::stable_sort(Ops.begin(), Ops.end(), Comp);
> }
>
>
>
> Modified: llvm/trunk/test/Analysis/ScalarEvolution/2008-07-29-SMinExpr.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvolution/2008-07-29-SMinExpr.ll?rev=105540&r1=105539&r2=105540&view=diff
> ==============================================================================
> --- llvm/trunk/test/Analysis/ScalarEvolution/2008-07-29-SMinExpr.ll (original)
> +++ llvm/trunk/test/Analysis/ScalarEvolution/2008-07-29-SMinExpr.ll Mon Jun 7 14:06:13 2010
> @@ -22,5 +22,5 @@
> ret i32 %j.0.lcssa
> }
>
> -; CHECK: backedge-taken count is (-2147483632 + ((-1 + (-1 * %x)) smax (-1 + (-1 * %y))))
> +; CHECK: backedge-taken count is (-2147483632 + ((-1 + (-1 * %y)) smax (-1 + (-1 * %x))))
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list