[llvm] r319522 - Recommit rL319407: [SROA] enable splitting for non-whole-alloca loads and stores

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 14 07:29:49 PST 2017


Hi Hiroshi,
this seems to be responsible for https://bugs.llvm.org/show_bug.cgi?id=35657
Can you please take a look?

--
Davide

On Thu, Nov 30, 2017 at 10:05 PM, Hiroshi Inoue via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: inouehrs
> Date: Thu Nov 30 22:05:05 2017
> New Revision: 319522
>
> URL: http://llvm.org/viewvc/llvm-project?rev=319522&view=rev
> Log:
> Recommit rL319407: [SROA] enable splitting for non-whole-alloca loads and stores
>
> Recommiting once reverted patch rL319407 after adding a check for bit vector size to avoid failures in some build bots.
>
>
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>     llvm/trunk/test/DebugInfo/X86/sroasplit-2.ll
>     llvm/trunk/test/Transforms/SROA/basictest.ll
>     llvm/trunk/test/Transforms/SROA/big-endian.ll
>
> Modified: llvm/trunk/lib/Transforms/Scalar/SROA.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=319522&r1=319521&r2=319522&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/SROA.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/SROA.cpp Thu Nov 30 22:05:05 2017
> @@ -30,6 +30,7 @@
>  #include "llvm/ADT/PointerIntPair.h"
>  #include "llvm/ADT/STLExtras.h"
>  #include "llvm/ADT/SetVector.h"
> +#include "llvm/ADT/SmallBitVector.h"
>  #include "llvm/ADT/SmallPtrSet.h"
>  #include "llvm/ADT/SmallVector.h"
>  #include "llvm/ADT/Statistic.h"
> @@ -4047,27 +4048,58 @@ bool SROA::splitAlloca(AllocaInst &AI, A
>    // First try to pre-split loads and stores.
>    Changed |= presplitLoadsAndStores(AI, AS);
>
> -  // Now that we have identified any pre-splitting opportunities, mark any
> -  // splittable (non-whole-alloca) loads and stores as unsplittable. If we fail
> -  // to split these during pre-splitting, we want to force them to be
> -  // rewritten into a partition.
> +  // Now that we have identified any pre-splitting opportunities,
> +  // mark loads and stores unsplittable except for the following case.
> +  // We leave a slice splittable if all other slices are disjoint or fully
> +  // included in the slice, such as whole-alloca loads and stores.
> +  // If we fail to split these during pre-splitting, we want to force them
> +  // to be rewritten into a partition.
>    bool IsSorted = true;
> -  for (Slice &S : AS) {
> -    if (!S.isSplittable())
> -      continue;
> -    // FIXME: We currently leave whole-alloca splittable loads and stores. This
> -    // used to be the only splittable loads and stores and we need to be
> -    // confident that the above handling of splittable loads and stores is
> -    // completely sufficient before we forcibly disable the remaining handling.
> -    if (S.beginOffset() == 0 &&
> -        S.endOffset() >= DL.getTypeAllocSize(AI.getAllocatedType()))
> -      continue;
> -    if (isa<LoadInst>(S.getUse()->getUser()) ||
> -        isa<StoreInst>(S.getUse()->getUser())) {
> -      S.makeUnsplittable();
> -      IsSorted = false;
> +
> +  uint64_t AllocaSize = DL.getTypeAllocSize(AI.getAllocatedType());
> +  const uint64_t MaxBitVectorSize = 1024;
> +  if (AllocaSize <= MaxBitVectorSize) {
> +    // If a byte boundary is included in any load or store, a slice starting or
> +    // ending at the boundary is not splittable.
> +    SmallBitVector SplittableOffset(AllocaSize + 1, true);
> +    for (Slice &S : AS)
> +      for (unsigned O = S.beginOffset() + 1;
> +           O < S.endOffset() && O < AllocaSize; O++)
> +        SplittableOffset.reset(O);
> +
> +    for (Slice &S : AS) {
> +      if (!S.isSplittable())
> +        continue;
> +
> +      if ((S.beginOffset() > AllocaSize || SplittableOffset[S.beginOffset()]) &&
> +          (S.endOffset() > AllocaSize || SplittableOffset[S.endOffset()]))
> +        continue;
> +
> +      if (isa<LoadInst>(S.getUse()->getUser()) ||
> +          isa<StoreInst>(S.getUse()->getUser())) {
> +        S.makeUnsplittable();
> +        IsSorted = false;
> +      }
>      }
>    }
> +  else {
> +    // We only allow whole-alloca splittable loads and stores
> +    // for a large alloca to avoid creating too large BitVector.
> +    for (Slice &S : AS) {
> +      if (!S.isSplittable())
> +        continue;
> +
> +      if (S.beginOffset() == 0 && S.endOffset() >= AllocaSize)
> +        continue;
> +
> +      if (isa<LoadInst>(S.getUse()->getUser()) ||
> +          isa<StoreInst>(S.getUse()->getUser())) {
> +        S.makeUnsplittable();
> +        IsSorted = false;
> +      }
> +    }
> +  }
> +
>    if (!IsSorted)
>      std::sort(AS.begin(), AS.end());
>
>
> Modified: llvm/trunk/test/DebugInfo/X86/sroasplit-2.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/sroasplit-2.ll?rev=319522&r1=319521&r2=319522&view=diff
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/X86/sroasplit-2.ll (original)
> +++ llvm/trunk/test/DebugInfo/X86/sroasplit-2.ll Thu Nov 30 22:05:05 2017
> @@ -21,7 +21,8 @@
>
>  ; Verify that SROA creates a variable piece when splitting i1.
>  ; CHECK:  call void @llvm.dbg.value(metadata i64 %outer.coerce0, metadata ![[O:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64)),
> -; CHECK:  call void @llvm.dbg.value(metadata i64 %outer.coerce1, metadata ![[O]], metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)),
> +; CHECK:  call void @llvm.dbg.value(metadata i32 {{.*}}, metadata ![[O]], metadata !DIExpression(DW_OP_LLVM_fragment, 64, 32)),
> +; CHECK:  call void @llvm.dbg.value(metadata i32 {{.*}}, metadata ![[O]], metadata !DIExpression(DW_OP_LLVM_fragment, 96, 32)),
>  ; CHECK:  call void @llvm.dbg.value({{.*}}, metadata ![[I1:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)),
>  ; CHECK-DAG: ![[O]] = !DILocalVariable(name: "outer",{{.*}} line: 10
>  ; CHECK-DAG: ![[I1]] = !DILocalVariable(name: "i1",{{.*}} line: 11
>
> Modified: llvm/trunk/test/Transforms/SROA/basictest.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SROA/basictest.ll?rev=319522&r1=319521&r2=319522&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/SROA/basictest.ll (original)
> +++ llvm/trunk/test/Transforms/SROA/basictest.ll Thu Nov 30 22:05:05 2017
> @@ -1615,13 +1615,13 @@ define i16 @PR24463() {
>  ; Ensure we can handle a very interesting case where there is an integer-based
>  ; rewrite of the uses of the alloca, but where one of the integers in that is
>  ; a sub-integer that requires extraction *and* extends past the end of the
> -; alloca. In this case, we should extract the i8 and then zext it to i16.
> +; alloca. SROA can split the alloca to avoid shift or trunc.
>  ;
>  ; CHECK-LABEL: @PR24463(
>  ; CHECK-NOT: alloca
> -; CHECK: %[[SHIFT:.*]] = lshr i16 0, 8
> -; CHECK: %[[TRUNC:.*]] = trunc i16 %[[SHIFT]] to i8
> -; CHECK: %[[ZEXT:.*]] = zext i8 %[[TRUNC]] to i16
> +; CHECK-NOT: trunc
> +; CHECK-NOT: lshr
> +; CHECK: %[[ZEXT:.*]] = zext i8 {{.*}} to i16
>  ; CHECK: ret i16 %[[ZEXT]]
>  entry:
>    %alloca = alloca [3 x i8]
> @@ -1695,3 +1695,28 @@ bb1:
>    call void @llvm.lifetime.end.p0i8(i64 2, i8* %0)
>    ret void
>  }
> +
> +define void @test28(i64 %v) #0 {
> +; SROA should split the first i64 store to avoid additional and/or instructions
> +; when storing into i32 fields
> +
> +; CHECK-LABEL: @test28(
> +; CHECK-NOT: alloca
> +; CHECK-NOT: and
> +; CHECK-NOT: or
> +; CHECK:      %[[shift:.*]] = lshr i64 %v, 32
> +; CHECK-NEXT: %{{.*}} = trunc i64 %[[shift]] to i32
> +; CHECK-NEXT: ret void
> +
> +entry:
> +  %t = alloca { i64, i32, i32 }
> +
> +  %b = getelementptr { i64, i32, i32 }, { i64, i32, i32 }* %t, i32 0, i32 1
> +  %0 = bitcast i32* %b to i64*
> +  store i64 %v, i64* %0
> +
> +  %1 = load i32, i32* %b
> +  %c = getelementptr { i64, i32, i32 }, { i64, i32, i32 }* %t, i32 0, i32 2
> +  store i32 %1, i32* %c
> +  ret void
> +}
>
> Modified: llvm/trunk/test/Transforms/SROA/big-endian.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SROA/big-endian.ll?rev=319522&r1=319521&r2=319522&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/SROA/big-endian.ll (original)
> +++ llvm/trunk/test/Transforms/SROA/big-endian.ll Thu Nov 30 22:05:05 2017
> @@ -83,19 +83,34 @@ entry:
>    store i16 1, i16* %a0i16ptr
>
>    store i8 1, i8* %a2ptr
> -; CHECK:      %[[mask1:.*]] = and i40 undef, 4294967295
> -; CHECK-NEXT: %[[insert1:.*]] = or i40 %[[mask1]], 4294967296
>
>    %a3i24ptr = bitcast i8* %a3ptr to i24*
>    store i24 1, i24* %a3i24ptr
> -; CHECK-NEXT: %[[mask2:.*]] = and i40 %[[insert1]], -4294967041
> -; CHECK-NEXT: %[[insert2:.*]] = or i40 %[[mask2]], 256
>
>    %a2i40ptr = bitcast i8* %a2ptr to i40*
>    store i40 1, i40* %a2i40ptr
> -; CHECK-NEXT: %[[ext3:.*]] = zext i40 1 to i56
> -; CHECK-NEXT: %[[mask3:.*]] = and i56 undef, -1099511627776
> -; CHECK-NEXT: %[[insert3:.*]] = or i56 %[[mask3]], %[[ext3]]
> +
> +; the alloca is splitted into multiple slices
> +; Here, i8 1 is for %a[6]
> +; CHECK: %[[ext1:.*]] = zext i8 1 to i40
> +; CHECK-NEXT: %[[mask1:.*]] = and i40 undef, -256
> +; CHECK-NEXT: %[[insert1:.*]] = or i40 %[[mask1]], %[[ext1]]
> +
> +; Here, i24 0 is for %a[3] to %a[5]
> +; CHECK-NEXT: %[[ext2:.*]] = zext i24 0 to i40
> +; CHECK-NEXT: %[[shift2:.*]] = shl i40 %[[ext2]], 8
> +; CHECK-NEXT: %[[mask2:.*]] = and i40 %[[insert1]], -4294967041
> +; CHECK-NEXT: %[[insert2:.*]] = or i40 %[[mask2]], %[[shift2]]
> +
> +; Here, i8 0 is for %a[2]
> +; CHECK-NEXT: %[[ext3:.*]] = zext i8 0 to i40
> +; CHECK-NEXT: %[[shift3:.*]] = shl i40 %[[ext3]], 32
> +; CHECK-NEXT: %[[mask3:.*]] = and i40 %[[insert2]], 4294967295
> +; CHECK-NEXT: %[[insert3:.*]] = or i40 %[[mask3]], %[[shift3]]
> +
> +; CHECK-NEXT: %[[ext4:.*]] = zext i40 %[[insert3]] to i56
> +; CHECK-NEXT: %[[mask4:.*]] = and i56 undef, -1099511627776
> +; CHECK-NEXT: %[[insert4:.*]] = or i56 %[[mask4]], %[[ext4]]
>
>  ; CHECK-NOT: store
>  ; CHECK-NOT: load
> @@ -104,11 +119,12 @@ entry:
>    %ai = load i56, i56* %aiptr
>    %ret = zext i56 %ai to i64
>    ret i64 %ret
> -; CHECK-NEXT: %[[ext4:.*]] = zext i16 1 to i56
> -; CHECK-NEXT: %[[shift4:.*]] = shl i56 %[[ext4]], 40
> -; CHECK-NEXT: %[[mask4:.*]] = and i56 %[[insert3]], 1099511627775
> -; CHECK-NEXT: %[[insert4:.*]] = or i56 %[[mask4]], %[[shift4]]
> -; CHECK-NEXT: %[[ret:.*]] = zext i56 %[[insert4]] to i64
> +; Here, i16 1 is for %a[0] to %a[1]
> +; CHECK-NEXT: %[[ext5:.*]] = zext i16 1 to i56
> +; CHECK-NEXT: %[[shift5:.*]] = shl i56 %[[ext5]], 40
> +; CHECK-NEXT: %[[mask5:.*]] = and i56 %[[insert4]], 1099511627775
> +; CHECK-NEXT: %[[insert5:.*]] = or i56 %[[mask5]], %[[shift5]]
> +; CHECK-NEXT: %[[ret:.*]] = zext i56 %[[insert5]] to i64
>  ; CHECK-NEXT: ret i64 %[[ret]]
>  }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare


More information about the llvm-commits mailing list