[PATCH] D43390: [DEBUGINFO] Add support for emission of the inlined strings.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:118
 
+static cl::opt<DefaultOnOff>
+DwarfInlinedStrings("dwarf-inlined-strings", cl::Hidden,
----------------
probinson wrote:
> Are you intending to add platform-specific defaulting within DwarfDebug?  If not, then this is a simple on/off switch that defaults to off.
Yes, this is what Eric Christofer suggested to do as the first step. I'll enable it for NVPTX target


================
Comment at: test/DebugInfo/Generic/inlined-strings.ll:38
+
+; CHECK-NOT: DW_form_str
+
----------------
probinson wrote:
> This is spelled incorrectly (I expect you want DW_FORM_strp), and only verifies that the form does not appear after the last positive matching string.
> 
> I would be more inclined to run the test twice, once with Enable and once with Disable, with checks along the lines of
> ```
> ENABLE-NOT: DW_FORM_str{{(p|x)}}
> ENABLE: DW_FORM_string
> ENABLE-NOT: DW_FORM_str{{(p|x)}}
> 
> DISABLE-NOT: DW_FORM_string
> DISABLE: DW_FORM_str{{p|x)}}
> DISABLE-NOT: DW_FORM_string
> ```
> That makes the test specific to the behavior of the option, and the forms it produces, omitting irrelevant concerns about tags and attributes.
1. I don't care about some particular form here, I'm just checking that there are no any strings at all, in any form.
2. There are a lot of tests already where this new option is disabled by default and they have checks for `DW_form_strp` and other forms.


Repository:
  rL LLVM

https://reviews.llvm.org/D43390