This is an archive of the discontinued LLVM Phabricator instance.

Recommitting r358783 and r358786 "[MS] Emit S_HEAPALLOCSITE debug info" with fixes for buildbot error (undefined assembler label).
ClosedPublic

Authored by akhuang on Apr 24 2019, 12:20 PM.

Details

Summary

This emits labels around heapallocsite calls and S_HEAPALLOCSITE debug
info in codeview. Currently only changes FastISel, so emitting labels still
needs to be implemented in SelectionDAG.

Event Timeline

akhuang created this revision.Apr 24 2019, 12:20 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 24 2019, 12:20 PM
akhuang updated this revision to Diff 196496.Apr 24 2019, 12:21 PM
  • remove added whitespace
rnk added inline comments.Apr 24 2019, 1:20 PM
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
1078

I think it's worth a comment explaining why some of these labels might not be defined. Basically, if the instruction gets replaced anywhere in the codegen pipeline, they won't be defined.
Also, it would be LLVM-y to invert the condition and use continue to reduce indentation:
https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code

llvm/lib/Target/X86/X86FastISel.cpp
3988

Please add a new test case that fails if this line of code is removed. I think you can start from this C++ code:

struct Foo {
  __declspec(allocator) virtual void *alloc();
};
void use_alloc(void*);
void do_alloc(Foo *p) {
  use_alloc(p->alloc());
}
akhuang updated this revision to Diff 196525.Apr 24 2019, 2:55 PM
  • Add test case and comment for undefined labels
akhuang marked 3 inline comments as done.Apr 24 2019, 2:59 PM
rnk accepted this revision.Apr 24 2019, 3:08 PM

lgtm, thanks!

This revision is now accepted and ready to land.Apr 24 2019, 3:08 PM
This revision was automatically updated to reflect the committed changes.