[llvm] r178793 - Reassociate: Avoid iterator invalidation.

[Shuxin Yang]

Thank you for fixing this bug.

Alternatively, we can set the capacity of the vector at very beginning. 
This way we are still able to
access the operand through pointer, instead of relatively more expensive 
"vector[other-vector[idx]".

Can I get the big testing case offline?

Thanks
Shuxin



On 4/4/13 2:15 PM, Benjamin Kramer wrote:
> Author: d0k
> Date: Thu Apr  4 16:15:42 2013
> New Revision: 178793
>
> URL: http://llvm.org/viewvc/llvm-project?rev=178793&view=rev
> Log:
> Reassociate: Avoid iterator invalidation.
>
> OpndPtrs stored pointers into the Opnd vector that became invalid when the
> vector grows. Store indices instead. Sadly I only have a large testcase that
> only triggers under valgrind, so I didn't include it.
>
> Modified:
>      llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
>
> Modified: llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp?rev=178793&r1=178792&r2=178793&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp Thu Apr  4 16:15:42 2013
> @@ -143,9 +143,13 @@ namespace {
>       //   So, if Rank(X) < Rank(Y) < Rank(Z), it means X is defined earlier
>       //   than Y which is defined earlier than Z. Permute "x | 1", "Y & 2",
>       //   "z" in the order of X-Y-Z is better than any other orders.
> -    struct PtrSortFunctor {
> -      bool operator()(XorOpnd * const &LHS, XorOpnd * const &RHS) {
> -        return LHS->getSymbolicRank() < RHS->getSymbolicRank();
> +    class PtrSortFunctor {
> +      ArrayRef<XorOpnd> A;
> +
> +    public:
> +      PtrSortFunctor(ArrayRef<XorOpnd> Array) : A(Array) {}
> +      bool operator()(unsigned LHSIndex, unsigned RHSIndex) {
> +        return A[LHSIndex].getSymbolicRank() < A[RHSIndex].getSymbolicRank();
>         }
>       };
>     private:
> @@ -1270,7 +1274,7 @@ Value *Reassociate::OptimizeXor(Instruct
>       return 0;
>   
>     SmallVector<XorOpnd, 8> Opnds;
> -  SmallVector<XorOpnd*, 8> OpndPtrs;
> +  SmallVector<unsigned, 8> OpndIndices;
>     Type *Ty = Ops[0].Op->getType();
>     APInt ConstOpnd(Ty->getIntegerBitWidth(), 0);
>   
> @@ -1281,7 +1285,7 @@ Value *Reassociate::OptimizeXor(Instruct
>         XorOpnd O(V);
>         O.setSymbolicRank(getRank(O.getSymbolicPart()));
>         Opnds.push_back(O);
> -      OpndPtrs.push_back(&Opnds.back());
> +      OpndIndices.push_back(Opnds.size() - 1);
>       } else
>         ConstOpnd ^= cast<ConstantInt>(V)->getValue();
>     }
> @@ -1290,13 +1294,14 @@ Value *Reassociate::OptimizeXor(Instruct
>     //  the same symbolic value cluster together. For instance, the input operand
>     //  sequence ("x | 123", "y & 456", "x & 789") will be sorted into:
>     //  ("x | 123", "x & 789", "y & 456").
> -  std::sort(OpndPtrs.begin(), OpndPtrs.end(), XorOpnd::PtrSortFunctor());
> +  std::sort(OpndIndices.begin(), OpndIndices.end(),
> +            XorOpnd::PtrSortFunctor(Opnds));
>   
>     // Step 3: Combine adjacent operands
>     XorOpnd *PrevOpnd = 0;
>     bool Changed = false;
>     for (unsigned i = 0, e = Opnds.size(); i < e; i++) {
> -    XorOpnd *CurrOpnd = OpndPtrs[i];
> +    XorOpnd *CurrOpnd = &Opnds[OpndIndices[i]];
>       // The combined value
>       Value *CV;
>   
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits