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

Paul Robinson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 16 11:04:26 PST 2018


probinson added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:118
 
+static cl::opt<DefaultOnOff>
+DwarfInlinedStrings("dwarf-inlined-strings", cl::Hidden,
----------------
Are you intending to add platform-specific defaulting within DwarfDebug?  If not, then this is a simple on/off switch that defaults to off.


================
Comment at: test/DebugInfo/Generic/inlined-strings.ll:38
+
+; CHECK-NOT: DW_form_str
+
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D43390





More information about the llvm-commits mailing list