[llvm-commits] [llvm] r164596 - in /llvm/trunk: lib/Transforms/Scalar/SROA.cpp test/Transforms/SROA/phi-and-select.ll

[Roman Divacky]

Chandler,

Something in the new SROA is breaking PowerPC selfhost a lot.

I know that selfhosted llvm at around r164553 had llvm-test 74 failures
but r164603 has 430.

By naive disabling what I think can cause this:

Index: lib/Transforms/Scalar/SROA.cpp
===================================================================
--- lib/Transforms/Scalar/SROA.cpp      (revision 164603)
+++ lib/Transforms/Scalar/SROA.cpp      (working copy)
@@ -1845,7 +1845,7 @@
       ElementSize = VecTy->getScalarSizeInBits() / 8;
     } else if (isIntegerPromotionViable(TD, NewAI.getAllocatedType(),
                                         P, I, E)) {
-      IntPromotionTy = cast<IntegerType>(NewAI.getAllocatedType());
+      //IntPromotionTy = cast<IntegerType>(NewAI.getAllocatedType());
     }
     bool CanSROA = true;
     for (; I != E; ++I) {


I am seeing only 66 failures.

Not sure if it proves anything but Duncan said there's some little-endian
hardcoding in integer promotion. PowerPC is LE.

I dont have any testcase. Do you plan to fix the SROA LE thing in near future?
Or can you provide quick'n'dirty patch how to disable it (other than mine)?

Thank you, Roman 

On Tue, Sep 25, 2012 at 10:03:40AM -0000, Chandler Carruth wrote:
> Author: chandlerc
> Date: Tue Sep 25 05:03:40 2012
> New Revision: 164596
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=164596&view=rev
> Log:
> Fix a case where SROA did not correctly detect dead PHI or selects due
> to chains or cycles between PHIs and/or selects. Also add a couple of
> really nice test cases reduced from Kostya's reports in PR13905 and
> PR13906. Both are fixed by this patch.
> 
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>     llvm/trunk/test/Transforms/SROA/phi-and-select.ll
> 
> Modified: llvm/trunk/lib/Transforms/Scalar/SROA.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=164596&r1=164595&r2=164596&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/SROA.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/SROA.cpp Tue Sep 25 05:03:40 2012
> @@ -522,8 +522,10 @@
>  
>    void insertUse(Instruction &I, int64_t Offset, uint64_t Size,
>                   bool IsSplittable = false) {
> -    // Completely skip uses which don't overlap the allocation.
> -    if ((Offset >= 0 && (uint64_t)Offset >= AllocSize) ||
> +    // Completely skip uses which have a zero size or don't overlap the
> +    // allocation.
> +    if (Size == 0 ||
> +        (Offset >= 0 && (uint64_t)Offset >= AllocSize) ||
>          (Offset < 0 && (uint64_t)-Offset >= Size)) {
>        DEBUG(dbgs() << "WARNING: Ignoring " << Size << " byte use @" << Offset
>                     << " which starts past the end of the " << AllocSize
> @@ -697,6 +699,9 @@
>      SmallVector<std::pair<Instruction *, Instruction *>, 4> Uses;
>      Visited.insert(Root);
>      Uses.push_back(std::make_pair(cast<Instruction>(*U), Root));
> +    // If there are no loads or stores, the access is dead. We mark that as
> +    // a size zero access.
> +    Size = 0;
>      do {
>        Instruction *I, *UsedI;
>        llvm::tie(UsedI, I) = Uses.pop_back_val();
> @@ -824,9 +829,9 @@
>    }
>  
>    void insertUse(Instruction &User, int64_t Offset, uint64_t Size) {
> -    // If the use extends outside of the allocation, record it as a dead use
> -    // for elimination later.
> -    if ((uint64_t)Offset >= AllocSize ||
> +    // If the use has a zero size or extends outside of the allocation, record
> +    // it as a dead use for elimination later.
> +    if (Size == 0 || (uint64_t)Offset >= AllocSize ||
>          (Offset < 0 && (uint64_t)-Offset >= Size))
>        return markAsDead(User);
>  
> 
> Modified: llvm/trunk/test/Transforms/SROA/phi-and-select.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SROA/phi-and-select.ll?rev=164596&r1=164595&r2=164596&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/SROA/phi-and-select.ll (original)
> +++ llvm/trunk/test/Transforms/SROA/phi-and-select.ll Tue Sep 25 05:03:40 2012
> @@ -327,3 +327,48 @@
>    %load = load i32* %a
>    ret i32 %load
>  }
> +
> +define i32 @PR13905() {
> +; Check a pattern where we have a chain of dead phi nodes to ensure they are
> +; deleted and promotion can proceed.
> +; CHECK: @PR13905
> +; CHECK-NOT: alloca i32
> +; CHECK: ret i32 undef
> +
> +entry:
> +  %h = alloca i32
> +  store i32 0, i32* %h
> +  br i1 undef, label %loop1, label %exit
> +
> +loop1:
> +  %phi1 = phi i32* [ null, %entry ], [ %h, %loop1 ], [ %h, %loop2 ]
> +  br i1 undef, label %loop1, label %loop2
> +
> +loop2:
> +  br i1 undef, label %loop1, label %exit
> +
> +exit:
> +  %phi2 = phi i32* [ %phi1, %loop2 ], [ null, %entry ]
> +  ret i32 undef
> +}
> +
> +define i32 @PR13906() {
> +; Another pattern which can lead to crashes due to failing to clear out dead
> +; PHI nodes or select nodes. This triggers subtly differently from the above
> +; cases because the PHI node is (recursively) alive, but the select is dead.
> +; CHECK: @PR13906
> +; CHECK-NOT: alloca
> +
> +entry:
> +  %c = alloca i32
> +  store i32 0, i32* %c
> +  br label %for.cond
> +
> +for.cond:
> +  %d.0 = phi i32* [ undef, %entry ], [ %c, %if.then ], [ %d.0, %for.cond ]
> +  br i1 undef, label %if.then, label %for.cond
> +
> +if.then:
> +  %tmpcast.d.0 = select i1 undef, i32* %c, i32* %d.0
> +  br label %for.cond
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits