[PATCH] [GVN] Use a simpler form of IRBuilder constructor.

[Daniel Berlin]

LGTM

On Thu, Jun 11, 2015 at 4:42 PM, Alexey Samsonov <vonosmas at gmail.com> wrote:
> Hi echristo, dberlin, dblaikie,
>
> A side effect of this change is that it IRBuilder now automatically
> created debug info locations for new instructions, which is the
> same as debug location of insertion point. This is fine for the
> functions in questions (GetStoreValueForLoad and
> GetMemInstValueForLoad), as they are used in two situations:
>   * GVN::processLoad, which tries to eliminate a load. In this case
>     new instructions would have the same debug location as the load they
>     eventually replace;
>   * MaterializeAdjustedValue, which adds new instructions to the end
>     of the basic blocks, which could later be used to replace the load
>     definition. In this case we don't yet know the way the load would
>     be eventually replaced (either by assembling the precomputed values
>     via PHI, or by using them directly), so just using the basic block
>     strategy seems to be reasonable. There is also a special case
>     in the code that *would* adjust the location of the last
>     instruction replacing the load definition to the location of the
>     load.
>
> http://reviews.llvm.org/D10405
>
> Files:
>   lib/Transforms/Scalar/GVN.cpp
>   test/Transforms/GVN/load-pre-nonlocal.ll
>
> Index: lib/Transforms/Scalar/GVN.cpp
> ===================================================================
> --- lib/Transforms/Scalar/GVN.cpp
> +++ lib/Transforms/Scalar/GVN.cpp
> @@ -1120,7 +1120,7 @@
>    uint64_t StoreSize = (DL.getTypeSizeInBits(SrcVal->getType()) + 7) / 8;
>    uint64_t LoadSize = (DL.getTypeSizeInBits(LoadTy) + 7) / 8;
>
> -  IRBuilder<> Builder(InsertPt->getParent(), InsertPt);
> +  IRBuilder<> Builder(InsertPt);
>
>    // Compute which bits of the stored value are being used by the load.  Convert
>    // to an integer type to start with.
> @@ -1217,7 +1217,7 @@
>    LLVMContext &Ctx = LoadTy->getContext();
>    uint64_t LoadSize = DL.getTypeSizeInBits(LoadTy)/8;
>
> -  IRBuilder<> Builder(InsertPt->getParent(), InsertPt);
> +  IRBuilder<> Builder(InsertPt);
>
>    // We know that this method is only called when the mem transfer fully
>    // provides the bits for the load.
> Index: test/Transforms/GVN/load-pre-nonlocal.ll
> ===================================================================
> --- test/Transforms/GVN/load-pre-nonlocal.ll
> +++ test/Transforms/GVN/load-pre-nonlocal.ll
> @@ -53,35 +53,55 @@
>  ; %1 is partially redundant if %0 can be widened to a 64-bit load.
>
>  ; CHECK-LABEL: define i32 @overaligned_load
> +; CHECK: if.then:
> +; CHECK:   %0 = load i64
> +; CHECK:   [[LSHR:%[0-9]+]] = lshr i64 %0, 32, !dbg [[LSHR_LOC:![0-9]+]]
> +; CHECK:   trunc i64 [[LSHR]] to i32
>  ; CHECK: if.end:
>  ; CHECK-NOT: %1 = load i32, i32*
> +; CHECK: [[LSHR_LOC]] = !DILocation(line: 101, column: 1, scope: !{{.*}})
>
>  define i32 @overaligned_load(i32 %a, i32* nocapture %b) {
>  entry:
> -  %cmp = icmp sgt i32 %a, 0
> -  br i1 %cmp, label %if.then, label %if.else
> +  %cmp = icmp sgt i32 %a, 0, !dbg !14
> +  br i1 %cmp, label %if.then, label %if.else, !dbg !14
>
>  if.then:
> -  %0 = load i32, i32* getelementptr inbounds (%struct.S1, %struct.S1* @s1, i64 0, i32 0), align 8, !tbaa !5
> -  br label %if.end
> +  %0 = load i32, i32* getelementptr inbounds (%struct.S1, %struct.S1* @s1, i64 0, i32 0), align 8, !tbaa !5, !dbg !15
> +  br label %if.end, !dbg !15
>
>  if.else:
> -  %arrayidx = getelementptr inbounds i32, i32* %b, i64 2
> -  store i32 10, i32* %arrayidx, align 4, !tbaa !5
> -  br label %if.end
> +  %arrayidx = getelementptr inbounds i32, i32* %b, i64 2, !dbg !16
> +  store i32 10, i32* %arrayidx, align 4, !tbaa !5, !dbg !16
> +  br label %if.end, !dbg !16
>
>  if.end:
>    %i.0 = phi i32 [ %0, %if.then ], [ 0, %if.else ]
>    %p.0 = phi i32* [ getelementptr inbounds (%struct.S1, %struct.S1* @s1, i64 0, i32 0), %if.then ], [ %b, %if.else ]
> -  %add.ptr = getelementptr inbounds i32, i32* %p.0, i64 1
> -  %1 = load i32, i32* %add.ptr, align 4, !tbaa !5
> -  %add1 = add nsw i32 %1, %i.0
> -  ret i32 %add1
> +  %add.ptr = getelementptr inbounds i32, i32* %p.0, i64 1, !dbg !17
> +  %1 = load i32, i32* %add.ptr, align 4, !tbaa !5, !dbg !17
> +  %add1 = add nsw i32 %1, %i.0, !dbg !17
> +  ret i32 %add1, !dbg !17
>  }
>
>  !1 = !{!2, !2, i64 0}
>  !2 = !{!"any pointer", !3, i64 0}
>  !3 = !{!"omnipotent char", !4, i64 0}
>  !4 = !{!"Simple C/C++ TBAA"}
>  !5 = !{!6, !6, i64 0}
>  !6 = !{!"int", !3, i64 0}
> +
> +!llvm.module.flags = !{!7, !8, !9}
> +!7 = !{i32 2, !"Dwarf Version", i32 4}
> +!8 = !{i32 2, !"Debug Info Version", i32 3}
> +!9 = !{i32 1, !"PIC Level", i32 2}
> +
> +!10 = !{}
> +!11 = !DISubroutineType(types: !10)
> +!12 = !DIFile(filename: "test.cpp", directory: "/tmp")
> +!13 = !DISubprogram(name: "test", scope: !12, file: !12, line: 99, type: !11, isLocal: false, isDefinition: true, scopeLine: 100, flags: DIFlagPrototyped, isOptimized: false, function: i32 (i32, i32*)* @overaligned_load, variables: !10)
> +!14 = !DILocation(line: 100, column: 1, scope: !13)
> +!15 = !DILocation(line: 101, column: 1, scope: !13)
> +!16 = !DILocation(line: 102, column: 1, scope: !13)
> +!17 = !DILocation(line: 103, column: 1, scope: !13)
> +
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/