Page MenuHomePhabricator

[LLVM] Remove wrong DBG_VALUE instruction with one operand in AArch64 test case
ClosedPublic

Authored by Jac1494 on Apr 16 2020, 10:19 AM.

Details

Summary

AArch64 test case llvm/test/CodeGen/AArch64/branch-target-enforcement.mir is checking for invalid DBG_VALUE instruction with one operand(DBG_VALUE $lr). And this DBG_VALUE instruction is echoed from test case it self only.

Correct format of DBG_VALUE is given in below link:
https://llvm.org/docs/SourceLevelDebugging.html#variable-locations-in-instruction-selection-and-mir

Diff Detail

Event Timeline

Jac1494 created this revision.Apr 16 2020, 10:19 AM
dsanders accepted this revision.Apr 16 2020, 10:28 AM

LGTM. I'll remove the workaround in mir-strip-debug once this lands

Thanks

This revision is now accepted and ready to land.Apr 16 2020, 10:28 AM

Hi @dsanders ,

Thank you for accepting this. I don't have commit access please commit this.

Hi @dsanders ,

Thank you for accepting this. I don't have commit access please commit this.

Sure. Would you like me to use the email address you use on bugzilla as the commit author? If not, could you supply an address to use?

Sure. Would you like me to use the email address you use on bugzilla as the commit author? If not, could you supply an address to use?

Yes,@dsanders
Please use same email address which i used in bugzilla.(jaydeepchauhan1494@gmail.com)
Thanks.

This revision was automatically updated to reflect the committed changes.
efriedma added inline comments.
llvm/test/CodeGen/AArch64/branch-target-enforcement.mir
343

I think this is breaking the point of the test: it's specifically supposed to be checking that DBG_VALUE instructions don't confuse the transform. (So the test doesn't really care what the DBG_VALUE is, but it does care that one exists.)

Probably not too important, but still.

danielkiss added inline comments.Apr 17 2020, 1:27 AM
llvm/test/CodeGen/AArch64/branch-target-enforcement.mir
343

For background please take a look: D74492
I prefer to fix the DBG_VALUE instead of removing.

dsanders added inline comments.Apr 27 2020, 3:41 PM
llvm/test/CodeGen/AArch64/branch-target-enforcement.mir
343

I don't mind if it comes back as a valid instruction but there's currently no DILocalVariable to attach to it (or other LLVM-IR level debug info) and the LLVM-IR function doesn't have any values we could turn into one.

The code in D74492 is using isMetaInstruction() rather than isDebugInstruction() so maybe there's another instruction that would serve the purpose without needing to add all the LLVM-IR level debug info