[PATCH] D85085: Fix debug_loc offset difference with basic block sections

[Sriraman Tallam via Phabricator via llvm-commits]

tmsriram marked an inline comment as done.
tmsriram added inline comments.


================
Comment at: llvm/test/DebugInfo/X86/basic-block-sections-debug-loclist-3.ll:12-16
+; CHECK-NEXT: [0x{{[0-9a-f]+}}, 0x{{[0-9a-f]+}}): DW_OP_reg3
+; CHECK-NEXT: DW_AT_name	("i")
+; CHECK-NEXT: DW_AT_decl_file	("/code/loclist_3.cc")
+; CHECK-NEXT: DW_AT_decl_line	(8)
+; CHECK-NEXT: DW_AT_type	(0x{{[0-9a-f]+}} "int")
----------------
dblaikie wrote:
> tmsriram wrote:
> > dblaikie wrote:
> > > I /think/ this test might be able to be made a bit more resilient to future unrelated DWARF changes (such as adding/removing/renaming unrelated attributes) with something like:
> > > ```
> > > CHECK-NEXT: ... reg3
> > > CHECK-NEXT: DW_AT
> > > CHECK-NOT: DW_TAG
> > > CHECK: _name ("i")
> > > ```
> > > I'm not 100% sure that CHECK will match both in the case when there's a DW_AT_name immediately after the DW_TAG_location, and when there's some other attributes between location and name.
> > > 
> > > We don't always go to this much bother - but at least you can probably remove the decl file/line/type - the name seems enough to verify that the test hasn't accidentally started matching the location of some other variable like 'tmp'
> > > 
> > > (consider this sort of change/design aspect for the other test cases too)
> > I removed file, line and type.  Seems like the other stuff is useful but if you think I should prune more, please lmk.
> Looks good - if you like, the test can be made more robust (in case new unrelated attributes are added in the future) by using `CHECK-NOT: DW_TAG` between attributes rather than using CHECK-NEXT (that way you'd still be checking that the attributes are all associated with the same tag - but allowing the possibility of other attributes).
> 
> It doesn't make the test resilient to changes in the order of attributes (you might be able to do that with something like:
> ```
> CHECK: DW_TAG_variable
> CHECK-DAG: DW_AT_location
> CHECK-DAG: DW_AT_name
> CHECK: DW_TAG
> ```
> But if you do that, I'm not sure if the CHECK-NEXT needed for the DW_AT_location would work (not sure how it interacts with CHECK-DAG))
Ack, My different incantations seem to be less stable than this one. If you feel strongly about this, I can give this another pass.


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

https://reviews.llvm.org/D85085