[PATCH] D45024: [DebugInfo] Add DILabel metadata and intrinsic llvm.dbg.label.

[Hsiangkai Wang via Phabricator via llvm-commits]

HsiangKai added inline comments.


================
Comment at: lib/IR/AsmWriter.cpp:1978
+  Printer.printString("name", N->getName());
+  Printer.printMetadata("file", N->getRawFile());
+  Printer.printInt("line", N->getLine());
----------------
aprantl wrote:
> I think file and line should just come from the DILocation. Is there a situation where the File in the DILabel would not match the one in the DILocation?
The line number in DILocation is used for Instruction’s location. The line number is DILabel is used for Label’s location. Their meanings are different. They are just happened to be the same.

I reviewed llvm.dbg.declare implementation. It seems that the line number in DILocalVariable is also redundant. I am not very clear why there existed redundant line information in DILocalVariable. The only reason I guess is they have different meanings in logic and the implementation is easier if metadata has line number in itself. Actually, the line number of variable is got from variable’s metadata instead of derived from its intrinsic’s DILocation.

There may be other reasons I did not aware. However, if it is really redundant. I think there should be another patch to review the design in all metadata instead of in this patch. I prefer to be consistent with other metadata design concept.


Repository:
  rL LLVM

https://reviews.llvm.org/D45024