[PATCH] D138869: [Docs][RFC] Add AMDGPU LLVM Extensions for Heterogeneous Debugging

Tue Nov 29 04:45:33 PST 2022

probinson added a comment.

This is a very large document and I admit I've only briefly skimmed it. But I wonder whether the redesign of DIExpr in terms of new DIOpXyz (instead of DW_OP_Xyz) is introducing too many trees to let the forest be visible.  Finding a way to split this up into more digestible and reviewable chunks would be really wonderful.  The DIExpr redesign in particular (I'd expect) would be incredibly intrusive to implement, and I think you'd need to demonstrate that this redesign is absolutely necessary (cannot be implemented with the existing DWARF-like expressions perhaps augmented with some new DW_OP_LLVM_yyy operators).

Based on the Dev Meeting talk, the explicit notions of fragment and lifetime sounded appealing, and worth looking at independent of any "heterogeneous" aspect.  AMDGPU's needs might have been the motivation, but the problems addressed by these notions are generic. I'd expect these solutions should be broadly applicable.  It's especially exciting to see ways forward to support multiple concurrent locations, something that already comes up as a problem and would greatly benefit the debugging experience even in a scalar or homogeneous environment.

================
Comment at: llvm/docs/AMDGPULLVMExtensionsForHeterogeneousDebugging.rst:44
+information expressions to reflect code transformations. The LLVM extensions’
+changes are possible as LLVM has no requirement for backwards compatibility, nor
+any requirement that the intermediate representation of debug information
----------------
I wouldn't go that far.  LLVM does in fact promise to be able to read older versions of bitcode, necessarily including older versions of debug-info metadata.  so at the very least you'd need to be able to auto-upgrade older metadata into the new format.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138869