Skip to content

Conversation

@cjdb
Copy link
Contributor

@cjdb cjdb commented Jul 18, 2024

No description provided.

@cjdb cjdb requested a review from slackito July 18, 2024 16:56
@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2024

@llvm/pr-subscribers-debuginfo

Author: Christopher Di Bella (cjdb)

Changes

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

1 Files Affected:

  • (modified) llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp (+1)
diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
index b9cf36a07846c..3d90c0a2c0de3 100644
--- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
+++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
@@ -3362,6 +3362,7 @@ void InstrRefBasedLDV::buildVLocValueMap(
       if (BlockLiveIn->Kind == DbgValue::VPHI)
         BlockLiveIn->Kind = DbgValue::Def;
       auto &[Var, DILoc] = DVMap.lookupDVID(VarID);
+      (void)DILoc;
       assert(BlockLiveIn->Properties.DIExpr->getFragmentInfo() ==
                  Var.getFragment() &&
              "Fragment info missing during value prop");

@cjdb cjdb merged commit 574dbe3 into llvm:main Jul 18, 2024
if (BlockLiveIn->Kind == DbgValue::VPHI)
BlockLiveIn->Kind = DbgValue::Def;
auto &[Var, DILoc] = DVMap.lookupDVID(VarID);
[[maybe_unused]] auto &[Var, DILoc] = DVMap.lookupDVID(VarID);
Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW/next time, I'd consider rolling this into the assert, maybe?

      assert(BlockLiveIn->Properties.DIExpr->getFragmentInfo() ==
                 DVMap.lookupDVID(VarID).first.getFragment() &&
               ...

(so the optimizer doesn't have to deal with the lookup code in nonasserts builds)
(and/or if the named variable helped with readability/was needed for other reasons, maybe using auto &VarID = ...(VarID).first; ? rather than needing to name an extra fully unused variable (DILoc))

@jmorse
Copy link
Member

jmorse commented Jul 21, 2024

(Thanks for catching this for me!)

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary: 

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250781
@cjdb cjdb deleted the unused branch September 13, 2024 21:22
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.

5 participants