Make StructurizeCFG preserve the debug locations of the branch instructions it introduces.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Small update to submit for review.
Check the following recordings to see the differences while debugging
- debugging after the patch, on a gpu, https://asciinema.org/a/8mZ7AWPtXccaRRRiMNHmE0VDP
- debugging before the patch, on a gpu, https://asciinema.org/a/oFX7YDmWx9xyAlQMYBM5Mabov
- debugging on a cpu (not affected by this patch), https://asciinema.org/a/3pib6TyoLBYSt1Zumh0edTAnE
Notice how the behaviour after the patch is applied matches what happens for the cpu case.
llvm/lib/Transforms/Scalar/StructurizeCFG.cpp | ||
---|---|---|
966 | Fix this: remove empty line |
llvm/lib/Transforms/Scalar/StructurizeCFG.cpp | ||
---|---|---|
869 | Why can't you just directly set the debug info when the terminator is created? |
llvm/lib/Transforms/Scalar/StructurizeCFG.cpp | ||
---|---|---|
869 | That was my original approach, however I struggled to make it always work. For example, in the function changeExit when the branch is created and appended to the basic-block, sometimes there is the old terminator that's going to be replaced, but sometims the basic-block is empty (when it's a new flow block created by getNextFlow). In the end, I picked the current strategy since it was the simpler one. |
llvm/lib/Transforms/Scalar/StructurizeCFG.cpp | ||
---|---|---|
555 | I think the current guidance is DebugLocs should be handled by const reference |
llvm/lib/Transforms/Scalar/StructurizeCFG.cpp | ||
---|---|---|
555 | I changed to use a const-ref in here and in the loop in setTerminatorDebugLoc . Indeed, internally it uses a TrackingMDRef so it'd better to avoid constructing a new one. Thanks ! |
LGTM (although I would be a bit happier if this was setting something at the creation point)
I think the current guidance is DebugLocs should be handled by const reference