[polly] r309490 - Remove Debug metadata from copied instruction to prevent Module verification failure

Tobias Grosser via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 31 05:52:01 PDT 2017


On Mon, Jul 31, 2017, at 14:43, Sanjay Srivallabh Singapuram via
llvm-commits wrote:
> Hello Tobias,
> 
> This is interesting ! I'll take a look at it.
> 
> My patch was meant only for instructions being copied into GPUModule and
> thought that it wouldn't affect IR meant for the CPU.I'll find a way to
> remove the debugLoc for IR bound for the GPUModule alone.
> 
> Anyways, why are we copying instructions in the first place ? Please move
> this to polly-dev if you feel it's more appropriate place.

Because we re-schedule them and consequently copy them into a different
loop structure.

Best,
Tobias

> 
> Thanks,
> Sanjay
> 
> On Mon, 31 Jul 2017 at 17:15 Tobias Grosser <tobias.grosser at inf.ethz.ch>
> wrote:
> 
> > I reverted your commit in r309556 to get the buildbots green. We should
> > discuss the issue and then try to re-commit as soon as the underlying
> > problem has been understood and resolved.
> >
> > Best,
> > Tobias
> >
> > On Mon, Jul 31, 2017, at 13:42, Tobias Grosser via llvm-commits wrote:
> > > Hi Sanjay,
> > >
> > > since this commit I see build failures on our AOSP buildbot:
> > >
> > >
> > http://lab.llvm.org:8011/builders/aosp-O3-polly-before-vectorizer-unprofitable/builds/205/steps/build-aosp/logs/stdio
> > >
> > > inlinable function call in a function with debug info must have a !dbg
> > > location
> > >   %p_call = tail call i32 @_Z6_BLENDiii(i32 %blend_type, i32 %p_conv21,
> > >   i32 %p_conv19)
> > >
> > > I don't have a test case for this yet, but maybe you can have a quick
> > > look!
> > >
> > > Best,
> > > Tobias
> > >
> > > On Sat, Jul 29, 2017, at 20:03, Singapuram Sanjay Srivallabh via
> > > llvm-commits wrote:
> > > > Author: singam-sanjay
> > > > Date: Sat Jul 29 11:03:49 2017
> > > > New Revision: 309490
> > > >
> > > > URL: http://llvm.org/viewvc/llvm-project?rev=309490&view=rev
> > > > Log:
> > > > Remove Debug metadata from copied instruction to prevent Module
> > > > verification failure
> > > >
> > > > Summary:
> > > > **Remove debug metadata from instruction to be copied to prevent the
> > > > source file's debug metadata being copied into GPUModule and eventually
> > > > failing Module verification and ASM string codegeneration.**
> > > >
> > > > When copying the instruction onto the Module meant for the GPU, debug
> > > > metadata attached to an instruction causes all related metadata to be
> > > > pulled into the Module, including the DICompileUnit, which is not
> > listed
> > > > in llvm.dbg.cu of the Module. This fails the verification of the
> > Module
> > > > and generation of the ASM string.
> > > >
> > > > The only debug metadata of the instruction, the DebugLoc, is unset by
> > > > this patch.
> > > >
> > > > Reviewers: grosser, bollu, Meinersbur
> > > >
> > > > Reviewed By: grosser, bollu
> > > >
> > > > Subscribers: pollydev
> > > >
> > > > Tags: #polly
> > > >
> > > > Differential Revision: https://reviews.llvm.org/D35630
> > > >
> > > > Added:
> > > >     polly/trunk/test/GPGPU/debug-metadata-leak.ll
> > > > Modified:
> > > >     polly/trunk/lib/CodeGen/BlockGenerators.cpp
> > > >
> > > > Modified: polly/trunk/lib/CodeGen/BlockGenerators.cpp
> > > > URL:
> > > >
> > http://llvm.org/viewvc/llvm-project/polly/trunk/lib/CodeGen/BlockGenerators.cpp?rev=309490&r1=309489&r2=309490&view=diff
> > > >
> > ==============================================================================
> > > > --- polly/trunk/lib/CodeGen/BlockGenerators.cpp (original)
> > > > +++ polly/trunk/lib/CodeGen/BlockGenerators.cpp Sat Jul 29 11:03:49
> > 2017
> > > > @@ -234,6 +234,14 @@ void BlockGenerator::copyInstScalar(Scop
> > > >      NewInst->replaceUsesOfWith(OldOperand, NewOperand);
> > > >    }
> > > >
> > > > +  // When copying the instruction onto the Module meant for the GPU,
> > > > +  // debug metadata attached to an instruction causes all related
> > > > +  // metadata to be pulled into the Module. This includes the
> > > > DICompileUnit,
> > > > +  // which will not be listed in llvm.dbg.cu of the Module since the
> > > > Module
> > > > +  // doesn't contain one. This fails the verification of the Module
> > and
> > > > the
> > > > +  // subsequent generation of the ASM string.
> > > > +  NewInst->setDebugLoc(llvm::DebugLoc());
> > > > +
> > > >    Builder.Insert(NewInst);
> > > >    BBMap[Inst] = NewInst;
> > > >
> > > >
> > > > Added: polly/trunk/test/GPGPU/debug-metadata-leak.ll
> > > > URL:
> > > >
> > http://llvm.org/viewvc/llvm-project/polly/trunk/test/GPGPU/debug-metadata-leak.ll?rev=309490&view=auto
> > > >
> > ==============================================================================
> > > > --- polly/trunk/test/GPGPU/debug-metadata-leak.ll (added)
> > > > +++ polly/trunk/test/GPGPU/debug-metadata-leak.ll Sat Jul 29 11:03:49
> > > > 2017
> > > > @@ -0,0 +1,104 @@
> > > > +; RUN: opt %loadPolly %s -polly-process-unprofitable
> > -polly-codegen-ppcg
> > > > -polly-acc-dump-kernel-ir \
> > > > +; RUN: | FileCheck --check-prefix=KERNEL-IR %s
> > > > +
> > > > +; REQUIRES: pollyacc
> > > > +
> > > > +; KERNEL-IR: define ptx_kernel void @FUNC_vec_add_1_SCOP_0_KERNEL_0(i8
> > > > addrspace(1)* %MemRef_arr, i32 %N) #0 {
> > > > +
> > > > +; The instruction marked <<<LeakyInst>>> is copied into the GPUModule,
> > > > +; with changes only to the parameters to access data on the device
> > > > instead of
> > > > +; the host, i.e., MemRef_arr becomes polly.access.cast.MemRef_arr.
> > Since
> > > > the
> > > > +; instruction is annotated with a DILocation, copying the instruction
> > > > also copies
> > > > +; the metadata into the GPUModule. This stops codegenerating the
> > > > ptx_kernel by
> > > > +; failing the verification of the Module in GPUNodeBuilder::finalize,
> > > > due to the
> > > > +; copied DICompileUnit not being listed in a llvm.dbg.cu which was
> > > > neither copied
> > > > +; nor created.
> > > > +;
> > > > +; https://reviews.llvm.org/D35630 removes this debug metadata before
> > the
> > > > +; instruction is copied to the GPUModule.
> > > > +;
> > > > +; vec_add_1.c:
> > > > +;      void vec_add_1(int N, int arr[N]) {
> > > > +;        int i=0;
> > > > +;        for( i=0 ; i<N ; i++) arr[i] += 1;
> > > > +;      }
> > > > +;
> > > > +source_filename = "vec_add_1.c"
> > > > +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> > > > +target triple = "x86_64-unknown-linux-gnu"
> > > > +
> > > > +define void @vec_add_1(i32 %N, i32* %arr) !dbg !7 {
> > > > +entry:
> > > > +  call void @llvm.dbg.value(metadata i32 %N, i64 0, metadata !13,
> > > > metadata !16), !dbg !17
> > > > +  call void @llvm.dbg.value(metadata i32* %arr, i64 0, metadata !14,
> > > > metadata !16), !dbg !18
> > > > +  call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !15,
> > > > metadata !16), !dbg !19
> > > > +  %tmp = sext i32 %N to i64, !dbg !20
> > > > +  br label %for.cond, !dbg !20
> > > > +
> > > > +for.cond:                                         ; preds = %for.inc,
> > > > %entry
> > > > +  %indvars.iv = phi i64 [ %indvars.iv.next, %for.inc ], [ 0, %entry ]
> > > > +  call void @llvm.dbg.value(metadata i32 undef, i64 0, metadata !15,
> > > > metadata !16), !dbg !19
> > > > +  %cmp = icmp slt i64 %indvars.iv, %tmp, !dbg !22
> > > > +  br i1 %cmp, label %for.body, label %for.end, !dbg !24
> > > > +
> > > > +for.body:                                         ; preds = %for.cond
> > > > +  %arrayidx = getelementptr inbounds i32, i32* %arr, i64 %indvars.iv,
> > > > !dbg !25
> > > > +  %tmp1 = load i32, i32* %arrayidx, align 4, !dbg !26, !tbaa !27
> > > > +  %add = add nsw i32 %tmp1, 1, !dbg !26    ;   <<<LeakyInst>>>
> > > > +  store i32 %add, i32* %arrayidx, align 4, !dbg !26, !tbaa !27
> > > > +  br label %for.inc, !dbg !25
> > > > +
> > > > +for.inc:                                          ; preds = %for.body
> > > > +  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1, !dbg !31
> > > > +  call void @llvm.dbg.value(metadata !2, i64 0, metadata !15, metadata
> > > > !16), !dbg !19
> > > > +  br label %for.cond, !dbg !32, !llvm.loop !33
> > > > +
> > > > +for.end:                                          ; preds = %for.cond
> > > > +  ret void, !dbg !35
> > > > +}
> > > > +
> > > > +declare void @llvm.dbg.declare(metadata, metadata, metadata)
> > > > +
> > > > +declare void @llvm.dbg.value(metadata, i64, metadata, metadata)
> > > > +
> > > > +
> > > > +!llvm.dbg.cu = !{!0}
> > > > +!llvm.module.flags = !{!3, !4, !5}
> > > > +!llvm.ident = !{!6}
> > > > +
> > > > +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1,
> > producer:
> > > > "clang version 5.0.0 (http://llvm.org/git/clang.git
> > > > 23e042ffe07a923db2dbebf4d2a3692c5a454fee) (
> > http://llvm.org/git/llvm.git
> > > > 39c5686a1f54884f12120927b1753a750fdb5e02)", isOptimized: true,
> > > > runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
> > > > +!1 = !DIFile(filename: "vec_add_1.c", directory: "/tmp")
> > > > +!2 = !{}
> > > > +!3 = !{i32 2, !"Dwarf Version", i32 4}
> > > > +!4 = !{i32 2, !"Debug Info Version", i32 3}
> > > > +!5 = !{i32 1, !"wchar_size", i32 4}
> > > > +!6 = !{!"clang version 5.0.0 (http://llvm.org/git/clang.git
> > > > 23e042ffe07a923db2dbebf4d2a3692c5a454fee) (
> > http://llvm.org/git/llvm.git
> > > > 39c5686a1f54884f12120927b1753a750fdb5e02)"}
> > > > +!7 = distinct !DISubprogram(name: "vec_add_1", scope: !1, file: !1,
> > > > line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1,
> > > > flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !12)
> > > > +!8 = !DISubroutineType(types: !9)
> > > > +!9 = !{null, !10, !11}
> > > > +!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
> > > > +!11 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !10, size:
> > 64)
> > > > +!12 = !{!13, !14, !15}
> > > > +!13 = !DILocalVariable(name: "N", arg: 1, scope: !7, file: !1, line:
> > 1,
> > > > type: !10)
> > > > +!14 = !DILocalVariable(name: "arr", arg: 2, scope: !7, file: !1, line:
> > > > 1, type: !11)
> > > > +!15 = !DILocalVariable(name: "i", scope: !7, file: !1, line: 2, type:
> > > > !10)
> > > > +!16 = !DIExpression()
> > > > +!17 = !DILocation(line: 1, column: 20, scope: !7)
> > > > +!18 = !DILocation(line: 1, column: 27, scope: !7)
> > > > +!19 = !DILocation(line: 2, column: 7, scope: !7)
> > > > +!20 = !DILocation(line: 3, column: 8, scope: !21)
> > > > +!21 = distinct !DILexicalBlock(scope: !7, file: !1, line: 3, column:
> > 3)
> > > > +!22 = !DILocation(line: 3, column: 15, scope: !23)
> > > > +!23 = distinct !DILexicalBlock(scope: !21, file: !1, line: 3, column:
> > 3)
> > > > +!24 = !DILocation(line: 3, column: 3, scope: !21)
> > > > +!25 = !DILocation(line: 3, column: 25, scope: !23)
> > > > +!26 = !DILocation(line: 3, column: 32, scope: !23)
> > > > +!27 = !{!28, !28, i64 0}
> > > > +!28 = !{!"int", !29, i64 0}
> > > > +!29 = !{!"omnipotent char", !30, i64 0}
> > > > +!30 = !{!"Simple C/C++ TBAA"}
> > > > +!31 = !DILocation(line: 3, column: 21, scope: !23)
> > > > +!32 = !DILocation(line: 3, column: 3, scope: !23)
> > > > +!33 = distinct !{!33, !24, !34}
> > > > +!34 = !DILocation(line: 3, column: 35, scope: !21)
> > > > +!35 = !DILocation(line: 4, column: 1, scope: !7)
> > > >
> > > >
> > > > _______________________________________________
> > > > llvm-commits mailing list
> > > > llvm-commits at lists.llvm.org
> > > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at lists.llvm.org
> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list