- Added support for DW_AT_picture_string/DW_AT_digit_count/DW_AT_decimal_sign/DW_AT_binary_scale/DW_AT_decimal_scale
- Added DIBTFlags to DIBasicType.
- Moved BigEndian/LittleEndian flags to DIBasicType from DIType.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Could you please
- as much as reasonable split this into smaller patches
- add bitcode upgrade tests where you are moving flags around? (Check for for existing .bc tests as an example for how to write upgrade tests)
llvm/include/llvm/BinaryFormat/Dwarf.h | ||
---|---|---|
137 | Could you please split this change out into a separate NFC commit? | |
llvm/include/llvm/IR/DebugInfoFlags.def | ||
63 | This patch is introducing a bitcode incompatibility, but I don't see code in MetadataLoader to upgrade the old format. |
Please do a separate patch that moves only the existing basic-type flags without introducing any new ones. Once you have that in place, adding new flags to the field should be fairly simple to review. But as Adrian suggests, guaranteeing proper compatibility with older bitcode is very important and it is best to implement that as a predecessor patch.
Also could you please explain what motivates this change? I think the decimal attributes have come up previously, but it was a while ago and I am forgetting the details.
llvm/include/llvm-c/DebugInfo.h | ||
---|---|---|
685 | I believe the backward-compatibility requirements for the C API are very strict, and this does not preserve backward compatibility. |
I have created a small patch to separate DIFlags for DIBasicType. https://reviews.llvm.org/D74470
I will modify this patch afterwards.
The use case of this patch is to generate optional dwarf attributes DW_AT_digit_count/DW_AT_decimal_scale/DW_AT_binary_scale/DW_AT_decimal_sign/DW_AT_picture_string for languages like cobol, PL1, basic data types with encoding such as DW_ATE_edited DW_ATE_numeric_string etc.
Thanks, I will look at that soon.
The use case of this patch is to generate optional dwarf attributes DW_AT_digit_count/DW_AT_decimal_scale/DW_AT_binary_scale/DW_AT_decimal_sign/DW_AT_picture_string for languages like cobol, PL1, basic data types with encoding such as DW_ATE_edited DW_ATE_numeric_string etc.
That's more a statement of the functionality of the patch than the use-case. You have a COBOL or PL/1 or BASIC compiler using LLVM?
These flags must remain, even if deprecated. Adding a comment explaining the situation is however warranted. See LLVMDIFlagFixedEnum for instance.