Skip to content

Conversation

abidh
Copy link
Contributor

@abidh abidh commented Sep 2, 2025

In the code that fix ups the debug information, we handles both the debug intrinsics and debug records. The debug intrinsics are being phased out and I recently changed mlir translation to not generate them. This means that we should not get debug intrinsics anymore and code can be simplified by removing their handling.

In the code that fixups the debug information, we handles both the
debug intrinsics and debug records. With recent changes in mlir
translation, we should not get debug intrinsics anymore. This
means we can simplify the code by removing the handling for debug
intrinsics and only focusing on debug records.
Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

LGTM

// The location and scope of variable intrinsics and records still point to
// the parent function of the target region. Update them.
for (Instruction &I : instructions(Func)) {
if (auto *DDI = dyn_cast<llvm::DbgVariableIntrinsic>(&I))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: do you think it is worth putting an assertion here that it is not an intrinsic to guard against these being used in the future?

I'm happy to go forward with or without the assertion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review. I have added an assert.

@abidh abidh merged commit cea2c86 into llvm:main Sep 2, 2025
9 checks passed
@abidh abidh deleted the dbg_intr_cleanup branch September 3, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants