flang icon indicating copy to clipboard operation
flang copied to clipboard

[DebugInfo] Use enhanced DIImportedEntity to generate better IR for renamed modules

Open alokkrsharma opened this issue 3 years ago • 18 comments

Consider the testcase below

module mymod
  integer :: var1 = 11
  integer :: var2 = 12
  integer :: var3 = 13
end module mymod
Program main
  call use_renamed()
  contains
    subroutine use_renamed()
      use mymod, var4 => var1
      print *, var4
    end subroutine use_renamed
end program main

Which currently produces IR as

!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
!1 = distinct !DIGlobalVariable(name: "var1", scope: !2, file: !3, line: 2, type: !9, isLocal: false, isDefinition: true)
!2 = !DIModule(scope: !4, name: "mymod", file: !3, line: 1)
!3 = !DIFile(filename: "usemodulealias.f90", directory: "/tmp")
!4 = distinct !DICompileUnit(language: DW_LANG_Fortran90, file: !3, producer: " F90 Flang - 1.5 2017-05-01", isOptimized: false, flags: "'+flang -g -S -emit-llvm usemodulealias.f90'", runtimeVersion: 0, emissionKind: FullDebug, enums: !5, retainedTypes: !5, globals: !6, **imports: !12**, nameTableKind: None)
!5 = !{}
!6 = !{!0, !7, !10}
!7 = !DIGlobalVariableExpression(var: !8, expr: !DIExpression(DW_OP_plus_uconst, 4))
!8 = distinct !DIGlobalVariable(name: "var2", scope: !2, file: !3, line: 3, type: !9, isLocal: false, isDefinition: true)
!9 = !DIBasicType(name: "integer", size: 32, align: 32, encoding: DW_ATE_signed)
!10 = !DIGlobalVariableExpression(var: !11, expr: !DIExpression(DW_OP_plus_uconst, 8))
!11 = distinct !DIGlobalVariable(name: "var3", scope: !2, file: !3, line: 4, type: !9, isLocal: false, isDefinition: true)
**!12 = !{!13, !19, !20}**
**!13 = !DIImportedEntity(tag: DW_TAG_imported_declaration**, scope: !14, entity: !11, file: !3, line: 10)
!14 = distinct !DISubprogram(name: "use_renamed", scope: !15, file: !3, line: 10, type: !18, scopeLine: 10, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !4)
!15 = distinct !DISubprogram(name: "main", scope: !4, file: !3, line: 7, type: !16, scopeLine: 7, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagMainSubprogram, unit: !4)
!16 = !DISubroutineType(cc: DW_CC_program, types: !17)
!17 = !{null}
!18 = !DISubroutineType(types: !17)
**!19 = !DIImportedEntity(tag: DW_TAG_imported_declaration**, scope: !14, entity: !8, file: !3, line: 10)
**!20 = !DIImportedEntity(tag: DW_TAG_imported_declaration, name: "var4"**, scope: !14, entity: !1, file: !3, line: 10)

which can be optimize to

!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
!1 = distinct !DIGlobalVariable(name: "var1", scope: !2, file: !3, line: 2, type: !9, isLocal: false, isDefinition: true)
!2 = !DIModule(scope: !4, name: "mymod", file: !3, line: 1)
!3 = !DIFile(filename: "usemodulealias.f90", directory: "/tmp")
!4 = distinct !DICompileUnit(language: DW_LANG_Fortran90, file: !3, producer: " F90 Flang - 1.5 2017-05-01", isOptimized: false, flags: "'+flang usemodulealias.f90 -g -S -emit-llvm'", runtimeVersion: 0, emissionKind: FullDebug, enums: !5, retainedTypes: !5, globals: !6, **imports: !12**, nameTableKind: None)
!5 = !{}
!6 = !{!0, !7, !10}
!7 = !DIGlobalVariableExpression(var: !8, expr: !DIExpression(DW_OP_plus_uconst, 4))
!8 = distinct !DIGlobalVariable(name: "var2", scope: !2, file: !3, line: 3, type: !9, isLocal: false, isDefinition: true)
!9 = !DIBasicType(name: "integer", size: 32, align: 32, encoding: DW_ATE_signed)
!10 = !DIGlobalVariableExpression(var: !11, expr: !DIExpression(DW_OP_plus_uconst, 8))
!11 = distinct !DIGlobalVariable(name: "var3", scope: !2, file: !3, line: 4, type: !9, isLocal: false, isDefinition: true)
**!12 = !{!13}**
**!13 = !DIImportedEntity(tag: DW_TAG_imported_module**, scope: !14, entity: !2, file: !3, line: 10, **elements: !19**)
!14 = distinct !DISubprogram(name: "use_renamed", scope: !15, file: !3, line: 10, type: !18, scopeLine: 10, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !4)
**!19 = !{!20}
!20 = !DIImportedEntity(tag: DW_TAG_imported_declaration, name: "var4"**, scope: !14, entity: !1, file: !3, line: 10)

It is dependent on https://github.com/flang-compiler/classic-flang-llvm-project/pull/76 https://github.com/flang-compiler/classic-flang-llvm-project/pull/83

alokkrsharma avatar Dec 20 '21 17:12 alokkrsharma

The CI is fixed. Please rebase to run CI.

kiranchandramohan avatar Jan 27 '22 14:01 kiranchandramohan

@alokkrsharma Can you address the CI failure?

/home/runner/work/flang/flang/tools/flang2/flang2exe/lldebug.cpp:2457:59: error: implicit conversion of NULL constant to 'LL_MDRef' (aka 'unsigned int') [-Werror,-Wnull-conversion]
        (child ? ll_create_flexible_md_node(db->module) : NULL);
                                                          ^~~~
                                                          0
/home/runner/work/flang/flang/tools/flang2/flang2exe/lldebug.cpp:2464:75: error: implicit conversion of NULL constant to 'LL_MDRef' (aka 'unsigned int') [-Werror,-Wnull-conversion]
                                                      child->entity_type, NULL);

kiranchandramohan avatar Mar 22 '22 23:03 kiranchandramohan

Looks like on ARM64 latest LLVM commits (needed) are not being picked up.

alokkrsharma avatar Apr 08 '22 14:04 alokkrsharma

Looks like on ARM64 latest LLVM commits (needed) are not being picked up.

@alokkrsharma Which commits are you referring too? Are they in release/11.x upstream? We can backport them to our release_11x in classic-flang-llvm-project.

bryanpkc avatar May 18 '22 10:05 bryanpkc

Oh never mind. I think we need to update the AArch64 CI jobs to build release_13x instead of release_11x.

bryanpkc avatar May 18 '22 11:05 bryanpkc

Can't restart the checks, can the original author re-trigger them?

pawosm-arm avatar Jul 13 '22 15:07 pawosm-arm

Can't restart the checks, can the original author re-trigger them?

Just restarted. Thanks.

alokkrsharma avatar Jul 13 '22 15:07 alokkrsharma

I suspect, usemodule.f90 must be marked with ! REQUIRES: llvm-13

pawosm-arm avatar Jul 13 '22 16:07 pawosm-arm

I suspect, usemodule.f90 must be marked with ! REQUIRES: llvm-13

Flang (front-end) produces new IR for release >= 11, as required changes in LLVM are available in release >=11 (https://github.com/flang-compiler/classic-flang-llvm-project/pull/76) . With this I expect that to work on failing tests for ARM. Is there something I am missing ? Does not ARM uses release_11x ?

alokkrsharma avatar Jul 24 '22 09:07 alokkrsharma

Does not ARM uses release_11x ?

Well, actually, not anymore

pawosm-arm avatar Jul 24 '22 09:07 pawosm-arm

Does not ARM uses release_11x ?

Well, actually, not anymore

then should we not suppress (stop testing) the failing check (AArch64, gcc, g++, 10, release_11x) ?

alokkrsharma avatar Jul 24 '22 09:07 alokkrsharma

Does not ARM uses release_11x ?

Well, actually, not anymore

then should we not suppress (stop testing) the failing check (AArch64, gcc, g++, 10, release_11x) ?

I'm working on that in PR #1274.

bryanpkc avatar Jul 27 '22 11:07 bryanpkc

If we can fix the CI in the next couple of days, I would prefer not to put REQUIRES: x86_64-linux on the test.

bryanpkc avatar Jul 27 '22 11:07 bryanpkc

If we can fix the CI in the next couple of days, I would prefer not to put REQUIRES: x86_64-linux on the test.

Thanks Bryan for working on CI, I'll wait for your fix and revert REQUIRES change.

alokkrsharma avatar Aug 01 '22 04:08 alokkrsharma

Please go ahead and rebase on the latest master.

bryanpkc avatar Aug 02 '22 17:08 bryanpkc

Please go ahead and rebase on the latest master.

Thanks @bryanpkc .

alokkrsharma avatar Aug 02 '22 17:08 alokkrsharma

Thanks to @bryanpkc , the check have passed finally. Hi @bryanpkc @kiranchandramohan @pawosm-arm can you please approve this ?

alokkrsharma avatar Aug 02 '22 17:08 alokkrsharma

Gentle reminder for review !

alokkrsharma avatar Aug 08 '22 14:08 alokkrsharma

Hi @bryanpkc , do you have any more comments ?

alokkrsharma avatar Aug 24 '22 06:08 alokkrsharma

LGTM. Thanks for the reminder.

Thanks a lot @bryanpkc

alokkrsharma avatar Aug 24 '22 10:08 alokkrsharma

Hi @kiranchandramohan @pawosm-arm, do you have any comments ?

alokkrsharma avatar Aug 24 '22 10:08 alokkrsharma