This is an archive of the discontinued LLVM Phabricator instance.

[DebugInfo] Replace UndefValue with PoisonValue in AssignmentTrackingAnalysis
ClosedPublic

Authored by Orlando on Jan 3 2023, 9:10 AM.

Diff Detail

Event Timeline

Orlando created this revision.Jan 3 2023, 9:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2023, 9:10 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
Orlando requested review of this revision.Jan 3 2023, 9:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2023, 9:10 AM

Code LGTM, thank you! One small request is to move some comments around and update llvm/docs/AssignmentTracking.md to reflect the change.

Something like:

The first three parameters look and behave like an llvm.dbg.value. ID is a reference to a store (see next section). Address is the destination address of the store and it is modified by AddressExpression. An empty/undef/poison Address represents <...>.
llvm/include/llvm/IR/IntrinsicInst.h
451 ↗(On Diff #486009)

I vote for a short doc comment mentioning that this is the only correct way to ask a DbgAssignIntrinsic whether it "kills" the address-based location.

llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
1481–1482

Somewhat stale reference to undef directly here. Could the comment just be folded into a general one on isAddressKillLocation itself?

Orlando updated this revision to Diff 486826.Jan 6 2023, 5:25 AM
Orlando marked 2 inline comments as done.

Thanks for the review!

+ Update the docs
+ Open new parent patch D141125 that adds is/setKillAddress

scott.linder accepted this revision.Jan 9 2023, 12:55 PM

LGTM, thank you!

This revision is now accepted and ready to land.Jan 9 2023, 12:55 PM
This revision was landed with ongoing or failed builds.Jan 12 2023, 2:11 AM
This revision was automatically updated to reflect the committed changes.