Skip to content

Conversation

@apolloww
Copy link
Contributor

@apolloww apolloww commented Jan 2, 2024

Follow up #75402 to cover DPValue

@llvmbot
Copy link
Member

llvmbot commented Jan 2, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Wei Wang (apolloww)

Changes

Follow up #75402 to cover DPValue


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Coroutines/CoroFrame.cpp (+8-2)
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp index 529f7309a1a273..690f813b477658 100644 --- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp +++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp @@ -2953,6 +2953,8 @@ void coro::salvageDebugInfo( std::optional<BasicBlock::iterator> InsertPt; if (auto *I = dyn_cast<Instruction>(Storage)) { InsertPt = I->getInsertionPointAfterDef(); + // Update DILocation only in O0 since it is easy to get out of sync in optimizations. + // See https://github.com/llvm/llvm-project/pull/75104 for an example. if (!OptimizeFrame && I->getDebugLoc()) DVI.setDebugLoc(I->getDebugLoc()); } else if (isa<Argument>(Storage)) @@ -2988,9 +2990,13 @@ void coro::salvageDebugInfo( // dbg.declare does. if (DPV.getType() == DPValue::LocationType::Declare) { std::optional<BasicBlock::iterator> InsertPt; - if (auto *I = dyn_cast<Instruction>(Storage)) + if (auto *I = dyn_cast<Instruction>(Storage)) { InsertPt = I->getInsertionPointAfterDef(); - else if (isa<Argument>(Storage)) + // Update DILocation only in O0 since it is easy to get out of sync in optimizations. + // See https://github.com/llvm/llvm-project/pull/75104 for an example. + if (!OptimizeFrame && I->getDebugLoc()) + DPV.setDebugLoc(I->getDebugLoc()); + } else if (isa<Argument>(Storage)) InsertPt = F->getEntryBlock().begin(); if (InsertPt) { DPV.removeFromParent(); 
@llvmbot
Copy link
Member

llvmbot commented Jan 2, 2024

@llvm/pr-subscribers-coroutines

Author: Wei Wang (apolloww)

Changes

Follow up #75402 to cover DPValue


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Coroutines/CoroFrame.cpp (+8-2)
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp index 529f7309a1a273..690f813b477658 100644 --- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp +++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp @@ -2953,6 +2953,8 @@ void coro::salvageDebugInfo( std::optional<BasicBlock::iterator> InsertPt; if (auto *I = dyn_cast<Instruction>(Storage)) { InsertPt = I->getInsertionPointAfterDef(); + // Update DILocation only in O0 since it is easy to get out of sync in optimizations. + // See https://github.com/llvm/llvm-project/pull/75104 for an example. if (!OptimizeFrame && I->getDebugLoc()) DVI.setDebugLoc(I->getDebugLoc()); } else if (isa<Argument>(Storage)) @@ -2988,9 +2990,13 @@ void coro::salvageDebugInfo( // dbg.declare does. if (DPV.getType() == DPValue::LocationType::Declare) { std::optional<BasicBlock::iterator> InsertPt; - if (auto *I = dyn_cast<Instruction>(Storage)) + if (auto *I = dyn_cast<Instruction>(Storage)) { InsertPt = I->getInsertionPointAfterDef(); - else if (isa<Argument>(Storage)) + // Update DILocation only in O0 since it is easy to get out of sync in optimizations. + // See https://github.com/llvm/llvm-project/pull/75104 for an example. + if (!OptimizeFrame && I->getDebugLoc()) + DPV.setDebugLoc(I->getDebugLoc()); + } else if (isa<Argument>(Storage)) InsertPt = F->getEntryBlock().begin(); if (InsertPt) { DPV.removeFromParent(); 
@apolloww
Copy link
Contributor Author

apolloww commented Jan 2, 2024

This addresses the build bot failure when -DLLVM_EXPERIMENTAL_DEBUGINFO_ITERATORS=ON: https://lab.llvm.org/buildbot/#/builders/275/builds/2885

@apolloww apolloww requested a review from OCHyams January 2, 2024 23:50
@github-actions
Copy link

github-actions bot commented Jan 2, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@OCHyams OCHyams left a comment

Choose a reason for hiding this comment

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

Thanks for updating the DPValue codepath. Looks like this matches the debug-intrinsic methodology, and the original pull request adds a test which includes --try-experimental-debuginfo-iterators, so this is already tested - LGTM.

@dyung
Copy link
Collaborator

dyung commented Jan 3, 2024

@apolloww Can you submit your change so that we can get the bot back to green?

@apolloww apolloww merged commit 0faf46b into llvm:main Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4 participants