Created attachment 23885 [details] IR for the unboxed-closures-unique-type-id.rs test We encountered this assertion in Rust x86_64-msvc trying to upgrade to LLVM 11. https://github.com/rust-lang/rust/pull/73526#issuecomment-678576555 llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp 1595 │ assert(!Subrange->getRawLowerBound() && 1596 │ "codeview doesn't support subranges with lower bounds"); The IR for that failed test is attached. It contains this: !157 = !DISubrange(count: 3, lowerBound: 0) The actual assertion was changed in D80197, specifically here: https://github.com/llvm/llvm-project/commit/d20bf5a7258d4b6a7f017a81b125275dac1aa166#diff-8412e58a7e99d6349b9f9418af573792 I confirmed that our IR passes the assertion before that commit, when it checked "Subrange->getLowerBound() == 0". I think that "!Subrange->getRawLowerBound()" is not equivalent though because that raw return value is a "Metadata *", so the assertion is now checking that it's NULL. This is different than having the lowerBound present with a constant value 0.
With the commit for DISubrange upgrade, there is change in supported types for lowerBound field of DISubrange and its internal representation. Earlier it supported integer type and its internal representation was integer. internal representation with zero was representing two things - lowerBound is absent (like DISubrange(count 3) Or, lowerBound is present with value zero (DISubrange(count: 3, lowerBound: 0) With upgrade to DISubrange, there are different internal representations for above two different cases. Now coming to current code line. llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp 1595 │ assert(!Subrange->getRawLowerBound() && 1596 │ "codeview doesn't support subranges with lower bounds"); the assertion comment "codeview doesn't support subranges with lower bounds", suggests it expects absent lowerBound, this is why new condition is so. Should not the failed IR be actually invalidated as per the assertion comment, the way it is happening now? In case it lowerBound present with value zero is valid IR for Codeview then we must also update the assertion comment to reflect this along with the condition. Please let me know your view.
I don't really know anything about codeview, but my impression is that "doesn't support lower bounds" really means that the lower bound is always implicitly zero, because the format doesn't have a way to specify anything else. If so, then it shouldn't be an error to have an explicit zero either. Rust is using DIBuilder::getOrCreateSubrange(int64_t Lo, int64_t Count) -- and before your change the other choice was only (int64_t Lo, Metadata *CountNode). I suppose now we could use your new one with 4 Metadata*, filling only the count and leaving the others NULL. But first I think we should get a codeview person to answer whether lowerBound:0 ought to be accepted here.
+some folks who can maybe help with the codeview angle
My impression is that the codeview code checked for `== 0` because the IR used to use lowerBound:0 to denote no lower bound. Based on Alok's comment we now use a null metadata to represent no lower bound, so using the new getOrCreateDISubrange seems like the right thing to do if there is no explicit lower bound.
Okay, sounds like there's no bug here then, but that the API has changed. Alok, could you send a patch to llvm/docs/ReleaseNotes.rst on the release/11.x branch that mentions this change? I think the "Changes to the LLVM IR" section would be a good spot.
All of the non-empty examples of DISubrange in the LangRef have a lowerBound, with a comment on the first, "array counting from 0". It seems strange to suggest that you must not specify any lowerBound for codeview. (In reply to Amy Huang from comment #4) > My impression is that the codeview code checked for `== 0` because the IR > used to use lowerBound:0 to denote no lower bound. I don't know what "no lower bound" would really *mean*. An indexable array should always have a first index, no? That may be an implied 0, and maybe codeview can't express anything else, but ISTM the concept always exists.
For comparison, the DWARF specification does allow DW_TAG_subrange_type to omit DW_AT_lower_bound, but then it assumes a default lower bound according to the current DW_AT_language -- table 7.17 in DWARF5.
Oh, I see. That makes sense, I agree that lower bound zero should also be ok for codeview.
Alok, can you help fix this?
(In reply to Hans Wennborg from comment #9) > Alok, can you help fix this? Adrian, you reviewed the patch above. Any ideas?
Okey, I shall push a patch to update the condition to accept "lowerBound: 0". I shall update the message as well.
(In reply to Alok Kumar Sharma from comment #11) > Okey, I shall push a patch to update the condition to accept "lowerBound: > 0". I shall update the message as well. Since now, lowerBound also supports Metadata and there can be 0 value (which is detectable at runtime). Should we not remove this assertion in place of updating it. Any suggestion ?
Please have a look at https://reviews.llvm.org/D87406
(In reply to Alok Kumar Sharma from comment #13) > Please have a look at > https://reviews.llvm.org/D87406 That was committed in e45b0708ae81 and merged to 11.x in 29d700a8132088ee6320702b601c0479a710a3ec Thanks!