Skip to content

[GSYM] Remove redundant getInliningInfoForAddress call #111136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

dstenb
Copy link
Collaborator

@dstenb dstenb commented Oct 4, 2024

In DwarfTransformer::verify() line number information is retrieved for
each address using:

auto DwarfInlineInfos =
DICtx.getInliningInfoForAddress(SectAddr, DLIS);

Later down the loop, another such invocation was made before:

Gsym->dump(Log, *FI);

There is a continue after that, DwarfInlineInfos do not affect the
dump() invocation, I am not aware of any other side effects that is
needed from the extra getInliningInfoForAddress() invocation, and tests
pass without it, so just remove it.

In DwarfTransformer::verify() line number information is retrieved for
each address using:

  auto DwarfInlineInfos =
      DICtx.getInliningInfoForAddress(SectAddr, DLIS);

Later down the loop, another such invocation was made before:

  Gsym->dump(Log, *FI);

There is a continue after that, DwarfInlineInfos do not affect the
dump() invocation, I am not aware of any other side effects that is
needed from the extra getInliningInfoForAddress() invocation, and tests
pass without it, so just remove it.
@llvmbot
Copy link
Member

llvmbot commented Oct 4, 2024

@llvm/pr-subscribers-debuginfo

Author: David Stenberg (dstenb)

Changes

In DwarfTransformer::verify() line number information is retrieved for
each address using:

auto DwarfInlineInfos =
DICtx.getInliningInfoForAddress(SectAddr, DLIS);

Later down the loop, another such invocation was made before:

Gsym->dump(Log, *FI);

There is a continue after that, DwarfInlineInfos do not affect the
dump() invocation, I am not aware of any other side effects that is
needed from the extra getInliningInfoForAddress() invocation, and tests
pass without it, so just remove it.


Full diff: https://github.com/llvm/llvm-project/pull/111136.diff

1 Files Affected:

  • (modified) llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp (-1)
diff --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index 894abf5777f161..3f5604e6aa4b06 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -699,7 +699,6 @@ llvm::Error DwarfTransformer::verify(StringRef GsymPath,
             Log << "    [" << Idx << "]: " << gii.Name << " @ " << gii.Dir
                 << '/' << gii.Base << ':' << gii.Line << '\n';
           }
-          DwarfInlineInfos = DICtx.getInliningInfoForAddress(SectAddr, DLIS);
           Gsym->dump(Log, *FI);
         }
         continue;

@dstenb dstenb requested review from clayborg and avl-llvm October 4, 2024 11:30
@dstenb
Copy link
Collaborator Author

dstenb commented Oct 15, 2024

Gentle ping.

Copy link
Collaborator

@dwblaikie dwblaikie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let's give it a go.

@dwblaikie dwblaikie merged commit 97da5e6 into llvm:main Oct 15, 2024
11 checks passed
@dstenb dstenb deleted the gsym_redundant_inline_info branch October 16, 2024 08:49
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
In DwarfTransformer::verify() line number information is retrieved for
each address using:

  auto DwarfInlineInfos =
      DICtx.getInliningInfoForAddress(SectAddr, DLIS);

Later down the loop, another such invocation was made before:

  Gsym->dump(Log, *FI);

There is a continue after that, DwarfInlineInfos do not affect the
dump() invocation, I am not aware of any other side effects that is
needed from the extra getInliningInfoForAddress() invocation, and tests
pass without it, so just remove it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants