Page MenuHomePhabricator

[DebugInfo] Introduce DIConstant metadata for representing named constants
AbandonedPublic

Authored by SouraVX on Feb 23 2021, 11:33 AM.

Details

Summary

LLVM Debug metadata lacks the capability of representing named constants. Languages such as Fortran has named constants, and code written in Fortran extensively uses Constants, hence there's an inherent need to provide debug information support for Constants to aid Fortran programs debugging.

This patch introduces DIConstant, which at the moment, will serve the needs of Fortran frontends.

Support Notes:

  • Only 64 bit Signed Integer supported for now.
  • Fortran programs with constants can can be debugged using GDB, correct value of constants getting accessed/printed.

    Testing:
    • check-llvm
    • check-llvm-unit (2 unit tests added).

Diff Detail

Unit TestsFailed

TimeTest
90 msx64 debian > Clang.Modules::debug-info-moduleimport.m
Script: -- : 'RUN: at line 1'; rm -rf /mnt/disks/ssd0/agent/llvm-project/build/tools/clang/test/Modules/Output/debug-info-moduleimport.m.tmp
930 msx64 windows > Clang.Modules::debug-info-moduleimport.m
Script: -- : 'RUN: at line 1'; rm -rf C:\ws\w1\llvm-project\premerge-checks\build\tools\clang\test\Modules\Output\debug-info-moduleimport.m.tmp

Event Timeline

SouraVX created this revision.Feb 23 2021, 11:33 AM
SouraVX requested review of this revision.Feb 23 2021, 11:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 23 2021, 11:33 AM
SouraVX updated this revision to Diff 325851.Feb 23 2021, 11:34 AM

Rebase + clang-format

SouraVX added inline comments.Feb 23 2021, 11:46 AM
llvm/test/Assembler/diconstant.ll
6

You may notice un-related value (168), this could be due to in-correct signed and unsigned interpretation ?

SouraVX updated this revision to Diff 326007.Feb 24 2021, 12:48 AM

Resolved the previous mentioned issue. Ready for review.

SouraVX retitled this revision from [WIP][DebugInfo] Introduce DIConstant metadata for representing named constants to [DebugInfo] Introduce DIConstant metadata for representing named constants.Feb 24 2021, 12:50 AM
SouraVX edited the summary of this revision. (Show Details)

Have you considered instead introducing a bool isConstant field in DILocalVariable? This way we could reuse all the machinery for local/global variables and only need to switch out the TAG in AsmPrinter.

Have you considered instead introducing a bool isConstant field in DILocalVariable? This way we could reuse all the machinery for local/global variables and only need to switch out the TAG in AsmPrinter.

Guess it gets a bit complicated if it has to support non-local constants too.

& could potentially do this via adding a tag field to DILocalVariable (& DIGlobalVariable too), same as for DICompositeTypes - but a bool's probably more compact & covers the use cases for now.

Guess it gets a bit complicated if it has to support non-local constants too.

We can already represent optimized out global variables with constant values as !DIGlobalVariableExpression(!DIGlobalVariable(...), !DIExpression(DW_OP_constu, ...)).
We would have to also add a bit to DIGlobalVariable, but other than that this seems perfectly feasible?

Guess it gets a bit complicated if it has to support non-local constants too.

We can already represent optimized out global variables with constant values as !DIGlobalVariableExpression(!DIGlobalVariable(...), !DIExpression(DW_OP_constu, ...)).
We would have to also add a bit to DIGlobalVariable, but other than that this seems perfectly feasible?

Yeah, I'm certainly OK with that, if it sounds good to you to do that in those two places.

Have you considered instead introducing a bool isConstant field in DILocalVariable? This way we could reuse all the machinery for local/global variables and only need to switch out the TAG in AsmPrinter.

Thanks for the review, @aprantl!
I considered this approach while hacking/iterating, but as mentioned here:
https://lists.llvm.org/pipermail/llvm-dev/2021-February/148447.html

A naïve attempt to represent it as GlobalVariable(or constant) as

!7 = !DIGlobalVariableExpression(var: !8, expr: !DIExpression(DW_OP_consts, 200))
!8 = distinct !DIGlobalVariable(name: "bar", scope: !2, file: !3, line: 3, type: !9, isLocal: false, isDefinition: true)

This will materialize in DWARF as:
0x0000004a:     DW_TAG_variable
                  DW_AT_name    ("bar")
                  ...
                 DW_AT_location        (DW_OP_addr 0x2007d4, DW_OP_consts +200)  // This is incorrect, pointing to data section

For DW_OP_consts case, it was creating an location entry like GlobalVariable, this wasn't making sense to me(at least), hence I thought of going this route.

Another motivating reason was: (well not an issue rather) introducing isConstant field is a bit non-trivial, considering tightly coupled nature of DILocalVariable and DIGlobalVariable with DIVariable. In my first attempt I introduced this isConstant field in DIVariable so that it can be utilized well for both situation. Obviously it worked - but broke some round tripping test cases.

I'll give it one more shot, if you have any ideas/suggestion WRT approach, please share I'll be happy to reason/iterate/implement.

Here's an example that seems to work:

source_filename = "globalconst.c"
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.13.0"

!llvm.dbg.cu = !{!2}
!llvm.module.flags = !{!10, !11, !12, !13}

!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
!3 = !DIFile(filename: "globalconst.c", directory: "/")
!4 = !{}
!5 = !{!16}
!7 = distinct !DIGlobalVariable(name: "constant", scope: !2, file: !3, line: 2, type: !8, isLocal: false, isDefinition: true)
!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
!10 = !{i32 2, !"Dwarf Version", i32 4}
!11 = !{i32 2, !"Debug Info Version", i32 3}
!12 = !{i32 1, !"wchar_size", i32 4}
!13 = !{i32 7, !"PIC Level", i32 2}
!16 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression(DW_OP_constu, 1, DW_OP_stack_value))
$ bin/llc -filetype=obj /tmp/globalconst.ll -o - | dwarfdump -
0x0000001e:   DW_TAG_variable
                DW_AT_name	("constant")
                DW_AT_type	(0x0000002a "int*")
                DW_AT_external	(true)
                DW_AT_decl_file	("globalconst.c")
                DW_AT_decl_line	(2)
                DW_AT_const_value	(1)
SouraVX added a comment.EditedMar 10 2021, 2:52 AM

Here's an example that seems to work:

...

Thanks @aprantl for quick example. Apologies for delayed response, I got occupied with something else.
One question I have is for Signed constants Consider slightly tweaked version of sample provide by you.
I tried representing 10(for demonstration purpose only) as:

!DIGlobalVariableExpression(var: !7, expr: !DIExpression(DW_OP_consts, 10, DW_OP_stack_value))

Compiling this, as you may notice, results in no warning/error, but DW_AT_location or DW_AT_const_value is missing from the DWARF so variable not visible in debugger.

source_filename = "globalconst.c"
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.13.0"

!llvm.dbg.cu = !{!2}
!llvm.module.flags = !{!10, !11, !12, !13}

!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
!3 = !DIFile(filename: "globalconst.c", directory: "/")
!4 = !{}
!5 = !{!16}
!7 = distinct !DIGlobalVariable(name: "constant", scope: !2, file: !3, line: 2, type: !8, isLocal: false, isDefinition: true)
!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
!10 = !{i32 2, !"Dwarf Version", i32 4}
!11 = !{i32 2, !"Debug Info Version", i32 3}
!12 = !{i32 1, !"wchar_size", i32 4}
!13 = !{i32 7, !"PIC Level", i32 2}
!16 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression(DW_OP_consts, 10, DW_OP_stack_value))  !Tweaked slightly, to check for negative constants
$ bin/llc -filetype=obj /tmp/globalconst.ll -o - | dwarfdump -
0x0000001e:   DW_TAG_variable
                DW_AT_name	("constant")
                DW_AT_type	(0x0000002a "int*")
                DW_AT_external	(true)
                DW_AT_decl_file	("globalconst.c")
                DW_AT_decl_line	(2)

Yeah that's just because

void DwarfCompileUnit::addLocationAttribute()

checks for

if (GlobalExprs.size() == 1 && Expr && Expr->isConstant()) {

and

bool DIExpression::isConstant() const {
  // Recognize DW_OP_constu C DW_OP_stack_value (DW_OP_LLVM_fragment Len Ofs)?.
  if (getNumElements() != 3 && getNumElements() != 6)
    return false;
  if (getElement(0) != dwarf::DW_OP_constu ||
      getElement(2) != dwarf::DW_OP_stack_value)
    return false;
  if (getNumElements() == 6 && getElement(3) != dwarf::DW_OP_LLVM_fragment)
    return false;
  return true;
}

We could probably just make addLocationAttribute() also accept DIExpression::isSignedConstant() and handle that appropriately.

SouraVX abandoned this revision.Wed, Mar 31, 12:38 AM