<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Sep 25, 2020, at 4:36 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">On Fri, Sep 25, 2020 at 4:08 PM Adrian Prantl <<a href="mailto:aprantl@apple.com" class="">aprantl@apple.com</a>> wrote:<br class=""><blockquote type="cite" class=""><br class="">First — thanks for fixing the test for me!<br class=""></blockquote><br class="">I'm a bit curious about the test - any idea how it came to be, if it's<br class="">invalid? Did we produce such bitcode in the past and don't anymore -<br class="">what's the rule about backwards compatibility here, then? It seems<br class="">like any time we regenerate a bitcode compat test that's questionable<br class="">because it means we won't be compatible with that bitcode we said we<br class="">should be compatible with?<br class=""></div></div></blockquote><div><br class=""></div><div>I think I mentioned in the commit message — we had a short period in clang with a bug</div><div><br class=""></div><div><p style="margin: 0px 0px 12px; padding: 0px; border: 0px;" class="">>>> Clang right before <a href="https://reviews.llvm.org/D79967" class="phui-tag-blue phui-tag-icon-view phui-tag-shade phui-tag-view phui-tag-type-shade" data-sigil="hovercard" data-meta="0_17" style="text-decoration: none; color: rgb(19, 108, 178); cursor: pointer; position: relative; -webkit-font-smoothing: antialiased; white-space: nowrap; margin-top: 0px;"><span class="phui-tag-core " style="color: rgb(70, 76, 92); border: 1px solid rgb(207, 219, 227); border-top-left-radius: 3px; border-top-right-radius: 3px; border-bottom-right-radius: 3px; border-bottom-left-radius: 3px; padding: 0px 4px; background-color: rgb(222, 231, 248); margin-top: 0px; background-position: initial initial; background-repeat: initial initial;"><span class="phui-font-fa visual-only fa-cog phui-icon-view" data-meta="0_16" aria-hidden="true" style="display: inline-block; color: rgb(107, 116, 140); line-height: 1; -webkit-font-smoothing: antialiased; margin: 0px 4px 0px 2px;"></span>https://reviews.llvm.org/D79967</span></a> would generate this kind of broken IR.</p><div class="">and the swift-5.3 branch happened to be cut in that unlucky window. To trigger it you need to markup a function with NoDebug (the bug was introduced maybe a month earlier).</div><div class=""><br class=""></div></div><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class="">We have two kinds of DISubprograms:<br class="">- uniqued DISubprograms are part of the type system and describe function *types*. They don't have a unit: field, because they want to be uniqued across compile units.<br class="">- distinct DISubprograms describe one specific function definition. They belong to a compile unit and thus have a unit: field.<br class=""><br class="">You are only supposed to attach distinct DISubprograms to function definitions. Several places in CodeGen will unconditionally dereference the unit: field and crash if it isn't there.<br class=""></blockquote><br class="">But do they have to be distinct? They have different fields (as you<br class="">say, declaration DISubprograms don't have a "unit:" field, definition<br class="">DISubprograms do have a unit: field that refers to a (itself distinct)<br class="">DICompileUnit)<br class=""></div></div></blockquote><div><br class=""></div><div>IIRC, DIBuilder lets you either create a distinct one with a unit or a uniqued one without. I don't know if the backend would survive the same uniqued DISubprogram node being attached to multiple llvm::Functions or if that would invalidate some assumptions... In any case distinct seems to be a reasonable proxy for what we mean.</div><div><br class=""></div><div>-- adrian</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class="">We actually had a weaker form of the check I added to Verifier already in place (<a href="https://github.com/llvm/llvm-project/blob/51cad041e0cb26597c7ccc0fbfaa349b8fffbcda/llvm/lib/IR/Verifier.cpp#L1186" class="">https://github.com/llvm/llvm-project/blob/51cad041e0cb26597c7ccc0fbfaa349b8fffbcda/llvm/lib/IR/Verifier.cpp#L1186</a>), but it didn't cover the specific testcase I added in <a href="https://reviews.llvm.org/rGe17f52d623cc146b7d9bf5a2e02965043508b4c4" class="">https://reviews.llvm.org/rGe17f52d623cc146b7d9bf5a2e02965043508b4c4</a>.<br class=""><br class="">-- adrian<br class=""></blockquote></div></div></blockquote></div><br class=""></body></html>