[PATCH] D79592: [DebugInfo] support for DW_AT_data_location in llvm

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:1393
+      if (Tag == dwarf::DW_TAG_array_type)
+        DataLocation = getMDOrNull(Record[17]);
     DICompositeType *CT = nullptr;
----------------
Is this the right thing to do in the ThinLTO case discussed above?


================
Comment at: llvm/test/DebugInfo/dwarfdump-dataLocationExp.ll:1
+;; This test checks DW_AT_data_location attribute
+
----------------
Could you expand the comment to describe what aspect of DW_AT_data_location this is testing?


================
Comment at: llvm/test/DebugInfo/dwarfdump-dataLocationExp.ll:3
+
+; RUN: llc %s -O2 -filetype=obj -o %t.o
+; RUN: llvm-dwarfdump  %t.o | FileCheck %s
----------------
Why do optimizations need to run in order to validate dwarfdump output? I'd expect it to be possible to write a test like this:

```
define void @f() {
  dbg.declare(%arrayVal, !arrayMD, ...)
  ret void
}
!arrayMD = !DILocalVariable(..., !typeMD)
!typeMD = !DICompositeType(array_type, ...)
```

Everything else makes the test harder to understand and maintain.


================
Comment at: llvm/test/DebugInfo/dwarfdump-dataLocationVar.ll:1
+;; This test checks DW_AT_data_location attribute
+
----------------
ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79592