[PATCH] D95518: [Debug-Info][XCOFF] support dwarf for XCOFF for assembly output

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:2323
+
+  // FIXME: use section end symbol as end of the Section. We need to consider
+  // the explicit sections and -ffunction-sections when we try to generate or
----------------
jasonliu wrote:
> I hope this FIXME is not going to stay here for long (and we will quickly have a patch to address this). As this is really a workaround here and it creates different behavior between object file generation and assembly generation which is undesirable.
I left this as a FIXME because I can not find an easy way to fix this. Why we need this function is because of the difference of the `changeSection` function of `MCAsmStreamer` and `MCObjectStreamer`. `MCObjectStreamer` will change the insert point to the section end, while `MCAsmStreamer` will add a new section start in the assembly output. The `MCAsmStreamer` current behavior is not what we need for debug lines, we can not add another `.text` section when we generate assembly for `.dwline`. 
So if we want to have the same behavior for `MCAsmStreamer` and `MCObjectStreamer`, we must make sure that we know a function is a section end. So we can add a section end symbol in the function `emitFunctionBodyEnd`. But this seems hard due to explicit sections. When we handle a function in `AsmPrinter` pass, it is hard to know there are other functions(which will be handled later) that also belong to the same section unless a CU level analysis is performed to fetch this info.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:2344
+    // Set to CU beginning.
+    emitIntValue(dwarf::DW_LNS_extended_op, 1);
+    emitULEB128IntValue(PointerSize + 1);
----------------
jasonliu wrote:
> The assembly generated is hard for human to consume. It will be very difficult to debug if anything goes wrong between those lines. It would be great if we could use addComment to let people know what each of this byte means (It could be in a follow on patch if this is too much for this one). 
> Some of the existing dwarf code does not really have those comments because they were meant for object file generation only. But now, it might make sense to have those comments available.
> Arguably, one could use some other tools to dump the assembled object file and figure out what each entry means. But those are extra steps and it requires time to map the dump output from object file to the original asm output. 
Please check the assembly generated now matching your expectation? Since the prologue of debug line is not newly generated by this patch, so I left it without comment to avoid many current cases fail.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:2341
+  if (!MAI->usesDwarfFileAndLocDirectives() && MMI->hasDebugInfo())
+    OutStreamer->doFinalizationAtSectionEnd(
+        OutStreamer->getContext().getObjectFileInfo()->getTextSection());
----------------
jasonliu wrote:
> I wonder if `doFinalizationAtSectionEnd` really has to be in MCStreamer. At least for this patch, it could just be a helper static function, or private member function of PPCAIXAsmPrinter or even a lambda here, and no other target actually needs it. We could always refactor it to a larger scope later when we find the need for it. 
I know what you mean, but we must implement this based on `MCStreamer`. We only need this for `MCAsmStreamer` but not for `MCObjectStreamer`. Currently, there is no way to differentiate these two streamers. And we should not differentiate them, as it breaks the abstraction of `MCSreamer`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95518/new/

https://reviews.llvm.org/D95518