<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On 16 Oct 2014, at 18:08, Eric Christopher <<a href="mailto:echristo@gmail.com" class="">echristo@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On Thu, Oct 16, 2014 at 6:06 PM, Duncan P. N. Exon Smith</span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class=""><</span><a href="mailto:dexonsmith@apple.com" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">dexonsmith@apple.com</a><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">> wrote:</span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""><blockquote type="cite" class="">On Oct 16, 2014, at 3:54 PM, Frederic Riss <<a href="mailto:friss@apple.com" class="">friss@apple.com</a>> wrote:<br class=""><br class="">If you look at the added testcase, it cast a void (*func)() to a int (*func)(). The int typed result is referenced in a debug info metadata node (an argument to the dbg.value intrinsic). Then instcombine would remove the bitcast and turn the int returning function into a void returning function that gets RAUWd over the value in the MDNode. And at this point in the current code you have invalid IR.<br class=""><br class="">The code in the testcase is reduced from a huge LTO link that encountered this situation in a much more complicated setup.<br class=""><br class=""><a href="http://reviews.llvm.org/D5828" class="">http://reviews.llvm.org/D5828</a><br class=""></blockquote><br class="">Weird. This LGTM (after fixing a few super minor whitespace changes);<br class="">not sure if Eric is still looking.<br class=""></blockquote><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Enh, I was trying to figure out if there was some way to avoid the</span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">void metadata node being create rather than avoiding RAUW'ing it. </span></div></blockquote><div><br class=""></div><div>When the MDNode is created it is perfectly legal (of type int) in this case. What we could do is special case the optimisation to call RAUW with a null value instead of the replacing void value. I’ve seen at least one place doing that. But I thought that preventing the illegal IR from being created altogether was a better choice.</div><div><br class=""></div><div>Fred</div><br class=""><blockquote type="cite" class=""><div class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">It's</span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">probably not a huge distinction at the moment.</span></div></blockquote><blockquote type="cite" class=""><div class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">-eric</span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""><blockquote type="cite" class="">Index: lib/IR/Metadata.cpp<br class="">===================================================================<br class="">--- lib/IR/Metadata.cpp<br class="">+++ lib/IR/Metadata.cpp<br class="">@@ -90,7 +90,11 @@<br class="">}<br class=""><br class="">void MDNodeOperand::allUsesReplacedWith(Value *NV) {<br class="">- getParent()->replaceOperand(this, NV);<br class="">+ // Void type operands aren't allowed anyway.<br class=""></blockquote><br class="">"anyway" is redundant. I'd just say:<br class=""><br class=""> // Void type operands aren't allowed.<br class=""><br class=""><blockquote type="cite" class="">+ if (NV->getType()->isVoidTy())<br class="">+ deleted();<br class="">+ else<br class="">+ getParent()->replaceOperand(this, NV);<br class="">}<br class=""><br class="">//===----------------------------------------------------------------------===//<br class="">Index: test/DebugInfo/voidMDNode.ll<br class="">===================================================================<br class="">--- /dev/null<br class="">+++ test/DebugInfo/voidMDNode.ll<br class="">@@ -0,0 +1,65 @@<br class="">+; RUN: opt -sroa -instcombine < %s | FileCheck %s<br class="">+<br class="">+; IR generated from (the obvisouly wrong):<br class=""></blockquote><br class="">s/obvisouly/obviously/<br class=""><br class=""><blockquote type="cite" class="">+; void foo(void) {}<br class="">+; void baz() {<br class="">+; int deadvar = ((int (*)(void))&foo)();<br class="">+; }<br class="">+<br class="">+; Function Attrs: nounwind ssp uwtable<br class="">+define void @foo() #0 {<br class="">+entry:<br class="">+ ret void, !dbg !17<br class="">+}<br class="">+<br class="">+; Function Attrs: nounwind ssp uwtable<br class="">+define void @baz() #0 {<br class="">+entry:<br class="">+ %deadvar = alloca i32, align 4<br class="">+; This dbg.declare will be turned in a dbg.value describing the bellow<br class="">+; store by sroa. The created dbg.value first argument will be the %call<br class="">+; value.<br class="">+ call void @llvm.dbg.declare(metadata !{i32* %deadvar}, metadata !18, metadata !19), !dbg !20<br class="">+ %call = call i32 bitcast (void ()* @foo to i32 ()*)(), !dbg !21<br class="">+; When instcombine removes the bitcast in the call instruction, it will<br class="">+; RAUW the %call value with the replacement instruction that is of void<br class="">+; type. Check that the dbg.value get's nullified rather that having a<br class="">+; bad reference to a void Value.<br class="">+;CHECK-NOT: <badref><br class=""></blockquote><br class="">Usually we have a space after the `;` for CHECKs.<br class=""><br class=""> ; CHECK-NOT: <badref><br class=""><br class=""><blockquote type="cite" class="">+ store i32 %call, i32* %deadvar, align 4, !dbg !21<br class="">+ ret void, !dbg !22<br class="">+}<br class="">+<br class="">+; Function Attrs: nounwind readnone<br class="">+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1<br class="">+<br class="">+attributes #0 = { nounwind ssp uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }<br class="">+attributes #1 = { nounwind readnone }<br class="">+<br class="">+!llvm.dbg.cu = !{!0}<br class="">+!llvm.module.flags = !{!14, !15}<br class="">+!llvm.ident = !{!16}<br class="">+<br class="">+!0 = metadata !{metadata !"0x11\0012\00clang version 3.6.0 \000\00\000\00\001", metadata !1, metadata !2, metadata !3, metadata !8, metadata !2, metadata !2} ; [ DW_TAG_compile_unit ] [/tmp/voidmetadata.c] [DW_LANG_C99]<br class="">+!1 = metadata !{metadata !"voidmetadata.c", metadata !"/tmp"}<br class="">+!2 = metadata !{}<br class="">+!3 = metadata !{metadata !4}<br class="">+!4 = metadata !{metadata !"0xf\00\000\0064\0064\000\000", null, null, metadata !5} ; [ DW_TAG_pointer_type ] [line 0, size 64, align 64, offset 0] [from ]<br class="">+!5 = metadata !{metadata !"0x15\00\000\000\000\000\000\000", null, null, null, metadata !6, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]<br class="">+!6 = metadata !{metadata !7}<br class="">+!7 = metadata !{metadata !"0x24\00int\000\0032\0032\000\000\005", null, null} ; [ DW_TAG_base_type ] [int] [line 0, size 32, align 32, offset 0, enc DW_ATE_signed]<br class="">+!8 = metadata !{metadata !9, metadata !13}<br class="">+!9 = metadata !{metadata !"0x2e\00foo\00foo\00\002\000\001\000\000\00256\000\002", metadata !1, metadata !10, metadata !11, null, void ()* @foo, null, null, metadata !2} ; [ DW_TAG_subprogram ] [line 2] [def] [foo]<br class="">+!10 = metadata !{metadata !"0x29", metadata !1} ; [ DW_TAG_file_type ] [/tmp/voidmetadata.c]<br class="">+!11 = metadata !{metadata !"0x15\00\000\000\000\000\000\000", null, null, null, metadata !12, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]<br class="">+!12 = metadata !{null}<br class="">+!13 = metadata !{metadata !"0x2e\00baz\00baz\00\004\000\001\000\000\000\000\004", metadata !1, metadata !10, metadata !11, null, void ()* @baz, null, null, metadata !2} ; [ DW_TAG_subprogram ] [line 4] [def] [baz]<br class="">+!14 = metadata !{i32 2, metadata !"Dwarf Version", i32 2}<br class="">+!15 = metadata !{i32 2, metadata !"Debug Info Version", i32 2}<br class="">+!16 = metadata !{metadata !"clang version 3.6.0 "}<br class="">+!17 = metadata !{i32 2, i32 17, metadata !9, null}<br class="">+!18 = metadata !{metadata !"0x100\00deadvar\005\000", metadata !13, metadata !10, metadata !7} ; [ DW_TAG_auto_variable ] [deadvar] [line 5]<br class="">+!19 = metadata !{metadata !"0x102"} ; [ DW_TAG_expression ]<br class="">+!20 = metadata !{i32 5, i32 6, metadata !13, null}<br class="">+!21 = metadata !{i32 5, i32 16, metadata !13, null}<br class="">+!22 = metadata !{i32 6, i32 1, metadata !13, null}</blockquote></blockquote></div></blockquote></div><br class=""></body></html>