Page MenuHomePhabricator

[NFC][DebugInfo] Factor LowerDbgDeclare into two functions [1/3]
AcceptedPublic

Authored by Orlando on Nov 13 2020, 7:16 AM.

Details

Summary

Setup work for https://llvm.org/PR47946. Split out the core functionality from
LowerDbgDeclare into a function called ConvertIndirectDbgIntrinsicToDbgValues
which will be used in subsequent patches.

This has been separated out from D91424 to help highlight the functional changes
in that patch.

Diff Detail

Event Timeline

Orlando created this revision.Nov 13 2020, 7:16 AM

This is fine apart from some nitpicks inside.

llvm/lib/Transforms/Utils/Local.cpp
1483–1484

Nit: remove LowerDbgDeclare -
Also, the doxygen comment should be in the header file and only there.

1484

Doxygen comment here to explain what this function does?
Which other indirect intrinsics are there?
I'm not sure if the name indirect captures the intention well unless it's part of a greater plan for the future. Otherwise I would just say DbgDeclare to avoid confusion.

Orlando edited the summary of this revision. (Show Details)EditedNov 23 2020, 2:13 AM

Thanks @aprantl for looking at this. I meant to state in the description that this NFC change is set up for D91424 to help highlight the functional change there.

llvm/lib/Transforms/Utils/Local.cpp
1483–1484

I left this as I found it, but I will happily update it.

1484

Naming this was a little tricky as in the next patch in this stack (D91424) the function is changed to work on dbg.value+derefs too.

Orlando updated this revision to Diff 316585.Jan 14 2021, 1:07 AM

Updated based on @aprantl's comments. I removed LowerDbgDeclare's doxygen comment from Local.cpp (it still exists in the header Local.h). I didn't add a doxygen comment to the new function ConvertIndirectDbgIntrinsicToDbgValues because it is added in the header in the next patch D91424.

aprantl accepted this revision.Jan 26 2021, 2:18 PM
This revision is now accepted and ready to land.Jan 26 2021, 2:18 PM