This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel][IRTranslator] Don't add debug info to constants emitted into the entry block
ClosedPublic

Authored by aemerson on Jun 13 2019, 10:32 AM.

Details

Summary

Constants, including G_GLOBAL_VALUE, are all emitted into the entry block which lets us use the vreg def assuming it dominates all other users. However, it can cause jumpy debug behaviour since the DebugLoc attached to these MIs are from
a user instruction that could be in a different block.

This makes GlobalISel consistent with SelectionDAG and fixes PR40887.

Diff Detail

Event Timeline

aemerson created this revision.Jun 13 2019, 10:32 AM
aprantl added inline comments.Jun 13 2019, 11:10 AM
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
1721

If you are inserting an instruction into the entry block without a DebugLoc and there is no instruction with a DebugLoc before it, it will get added to the function prologue. This may not matter, but I just wanted to make sure that you are aware of that.

aprantl added a project: debug-info.
aemerson marked an inline comment as done.Jun 13 2019, 11:18 AM
aemerson added inline comments.
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
1721

Thanks for the heads up. That seems reasonable to me.

dsanders added inline comments.Jun 13 2019, 11:19 AM
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
148

Should we limit this exception to G_CONSTANT and similar? Normal code can potentially be in the entry block too can't it?

1721

Does having a DebugLoc with line 0 have the same issue?

aprantl added inline comments.Jun 13 2019, 12:47 PM
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
1721

No, line 0 would force the prologue end like any other valid location.

aemerson updated this revision to Diff 204613.Jun 13 2019, 1:03 PM

Use a debug loc with line 0 instead, fix a test that should have been using fastisel.

This revision is now accepted and ready to land.Jun 13 2019, 2:55 PM
This revision was automatically updated to reflect the committed changes.