We have line 0 inherit the scope of the previous non-zero line, to limit the size cost of the line 0 records; but that scope might have a discriminator attached, and a discriminator on line 0 is pointless.
+Dehao to check on the semantics of this
|7–15 ↗||(On Diff #113493)|
Looks like test/DebugInfo/X86/discriminator2.ll might provide a simpler example code that creates discriminators using distinct call sites on the same source line? (maybe not - but was hoping for something a bit simpler/more direct than the code here)
The change looks good to me. But the test is quite large and hard to understand. I don't think it's necessary to have the test generated from a source file, just forge a test with discriminator attached to line 0, and then checks if discriminators is omitted in the generated code. I guess it should suffice with < 20 LOC
If the IR explicitly has a line 0 location with a nonzero discriminator, I think DwarfDebug should obediently emit that (or assert).
The point of this patch is that when DwarfDebug *implicitly* emits a line 0 record, a preceding discriminator should not "pollute" that.
Seems OK to me
For a minor perk - would it be possible to test the line table length? Since the benefit (smaller line table) of this change isn't readily apparent in the high level line table printing that dwarfdump does. I know it'd make the test a bit more brittle.
Yeah... I can add checks for both total_length and prologue_length, with a comment that says what we really care about is that the difference doesn't change unexpectedly. The metadata is specifying DWARF 4 so it shouldn't be *that* brittle.