The patch fixes emitting the offset to the type DIE. All other fields are already fixed in previous patches.
Details
Diff Detail
Event Timeline
Looks good.
General feedback: If you can include the original C source code and whatever compile command was needed to generate the IR in any tests (this includes other patches I've already approved, if possible - not vital, many of the tests have very small IR that's pretty easy to read, but it's a "nice to have") that'd be handy! Makes it easier to eyeball the dump output and understand what it's expecting.
The test sources were based on some simple code, but then they were cleared as much as I could. I guess they may be considered artificial. Do you think it will be beneficial to add the code like the following here?
struct Foo {int a;}; Foo foo;
You mean the IR was modified after compilation? Or that the original C source was simplified as much as possible?
Usually we don't simplify the IR - not worth it/doesn't make things especially more maintainable & more likely to introduce something that makes the IR quirky/not representative of the normal Clang generated IR.
I guess they may be considered artificial. Do you think it will be beneficial to add the code like the following here?
struct Foo {int a;}; Foo foo;
Yep! Nice and simple bits of stuff like that are great/handy to have!
I removed/adjusted some attributes here and there to make the tests smaller and to express mostly what I wanted to test in each particular case. In some cases, like the test for .debug_macro, the source was created fully manually, based on existing tests.
Usually we don't simplify the IR - not worth it/doesn't make things especially more maintainable & more likely to introduce something that makes the IR quirky/not representative of the normal Clang generated IR.
I'll see if I can update the tests in the series so that the IR is fully generated by clang.
Cool cool - use your best judgment, if the only pure Clang IR you can get is way more complicated than the hand-crafted/tweaked version (and the latter is simple enough in absolute, not just relative, terms to be fairly legible by itself) - the hand-crafted/tweaked version might be the better call. I imagine for macros this may be the case. Though I /thought/ I made it so a compile unit couldn't be emitted if it had no code/data in it - if that's the case, I'm not sure there's much simplification available in a macro test case compared to what clang would generate (& I'd use the simplest thing I can imagine to keep the CU live: "extern int i; int i;") - but if it can be done even without that, great!
clang-tidy: error: no member named 'getDwarfOffsetByteSize' in 'llvm::AsmPrinter' [clang-diagnostic-error]
not useful
clang-tidy: error: no member named 'getDwarfOffsetByteSize' in 'llvm::AsmPrinter' [clang-diagnostic-error]
not useful