[PATCH] D89218: [DebugInfo] Support for DW_TAG_generic_subrange

[Alok Kumar Sharma via Phabricator via llvm-commits]

alok marked 4 inline comments as done.
alok added inline comments.


================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:385
+  Metadata *getRawCountNode() const { return getOperand(0).get(); }
+
+  Metadata *getRawLowerBound() const { return getOperand(1).get(); }
----------------
djtodoro wrote:
> I think we can remove this newlines.
Thanks. I shall include this in next revision of patch.


================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:395
+  BoundType getCount() const;
+
+  BoundType getLowerBound() const;
----------------
djtodoro wrote:
> Here as well.
Thanks. I shall include this in next revision of patch.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1420
+  addBoundTypeEntry(dwarf::DW_AT_lower_bound, GSR->getLowerBound());
+
+  addBoundTypeEntry(dwarf::DW_AT_count, GSR->getCount());
----------------
djtodoro wrote:
> We don't need these extra newlines.
Thanks. I shall include this in next revision of patch.


================
Comment at: llvm/lib/IR/LLVMContextImpl.h:386
+  unsigned getHashValue() const {
+    if (CountNode)
+      if (auto *MD = dyn_cast<ConstantAsMetadata>(CountNode))
----------------
djtodoro wrote:
> I suggest:
>   auto *MD = dyn_cast<ConstantAsMetadata>(CountNode);
>   if (CountNode && MD)
>     return hash_combine(cast<ConstantInt>(...);
>   return hash_combine(CountNode,...);
Thanks. I shall include this in next revision of patch.


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

https://reviews.llvm.org/D89218