[PATCH] D92412: [DebugInfo] Add handling of stringLengthExp operand of DIStringType

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX added a comment.

Thanks @cchen15 for the patch! It seems like you're also adding support for `FORTRAN deferred length string` would you please make appropriate changes in summary to reflect the intent.



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:696
 
   if (DIVariable *Var = STy->getStringLength()) {
     if (auto *VarDIE = getDIE(Var))
----------------
cchen15 wrote:
> Can we consolidate stringLength and stringLengthExp into one operand? It seems to me that a DIStringType can have one or the other, but not both.
Thanks! makes sense in general. (As a side note, just please make sure we don't accidentally mess/touch up exisiting `assumed length string` support).


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:702
+    DIEDwarfExpression DwarfExpr(*Asm, getCU(), *Loc);
+    DwarfExpr.setMemoryLocationKind();
+    DwarfExpr.addExpression(Expr);
----------------
aprantl wrote:
> This would benefit from a comment explaining why forcing MemoryLocationKind is correct here.
+1 Please add a comment.

In FORTRAN deferred length string; string length is not known, that's why memory location I suppose ?
i.e 
`character(len=:), allocatable :: deferred`, this will be allocated later. 
`allocate(character(10)::deferred)`


================
Comment at: llvm/test/DebugInfo/X86/distringtype.ll:17
 
 ;; sample fortran testcase involving assumed length string type.
 ;; program assumedLength
----------------
NIT: Since the test case also contains FORTRAN deferred length string too.
Please update the comment like or
`....assumed length and deferred length string type.`


================
Comment at: llvm/test/DebugInfo/X86/distringtype.ll:19
 ;; program assumedLength
+;;   character(len:), allocatable :: deferred
+;;   allocate(character(10)::deferred)
----------------
NIT: missing`=`
`character(len=:), allocatable :: deferred`



================
Comment at: llvm/test/DebugInfo/fortran-string-type.ll:30
+!14 = !DIStringType(name: ".str.DEFERRED", stringLengthExpression: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 8))
\ No newline at end of file

----------------
NIT: Probably editor related ?


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

https://reviews.llvm.org/D92412