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