[PATCH] D113142: [CodeView] Properly handle a DISubprogram in getScopeIndex.

[Chih-Ping Chen via Phabricator via llvm-commits]

cchen15 added inline comments.


================
Comment at: llvm/test/DebugInfo/COFF/fortran-contained-proc.ll:25
+;
+; CHECK: S_GPROC32_ID [size = {{.*}}] `IF_TEST`
+; CHECK-NEXT: parent
----------------
rnk wrote:
> I think the test should look at IF_TEST_ip_SUB, since that's the nested routine, not the main routine.  The nested routine should use the main routine as its parent scope, right?
IF_TEST (the parent scope of SUB) is the subroutine I'd like to check here, because it is the one that getScopeIndex is called with, when the FuncId for SUB is being looked up in getFuncIdForSubprogram. And the test here is to ensure that IF_TEST points at the right type (two lines below).

That being said, your question prompted me to look at the rest of the dump a bit closer. Here are some observations:
1. SUB is inlined in the test. The S_INLINESITE sits nicely inside S_GPROC32_ID for IF_TEST. The type for that S_INLINESITE points at a LF_FUNC_ID whose parent scope points at the LF_FUNC_ID of IF_TEST.

```
0x1000 | LF_ARGLIST [size = 8]
0x1001 | LF_PROCEDURE [size = 16]
         return type = 0x0003 (void), # args = 0, param list = 0x1000
         calling conv = cdecl, options = None
0x1002 | LF_FUNC_ID [size = 20]
         name = IF_TEST, type = 0x1001, parent scope = <no type>
0x1003 | LF_FUNC_ID [size = 16]
         name = SUB, type = 0x1001, parent scope = 0x1002
<elided>
       0 | S_GPROC32_ID [size = 48] `IF_TEST`
           parent = 0, end = 0, addr = 0000:0000, code size = 69
           type = `0x1002 (IF_TEST)`, debug start = 0, debug end = 0, flags = none
       0 | S_FRAMEPROC [size = 32]
           size = 40, padding size = 0, offset to padding = 0
           bytes of callee saved registers = 0, exception handler addr = 0000:0000
           local fp reg = RSP, param fp reg = RSP
           flags = opt speed
       0 | S_LDATA32 [size = 16] `A`
           type = 0x0674 (int*), addr = 0000:0000
       0 | S_INLINESITE [size = 24]
           inlinee = 0x1003 (SUB), parent = 0, end = 0
             0608      line 4 (+4)
             033A      code 0x3A (+0x3A)
             0406      code end 0x40 (+0x6)
             0000

```

2. There is a separate S_GPROC32_ID emitted for IF_TEST::SUB. with its parent set to 0:

```
       0 | S_GPROC32_ID [size = 52] `IF_TEST::SUB`
           parent = 0, end = 0, addr = 0000:0000, code size = 7
           type = `0x1003 (SUB)`, debug start = 0, debug end = 0, flags = none
```

1. looks fine but I am not sure about 2. I will look into this further. If you have any insight, please share with me.

Note, however, the entry in 2. is also emitted before the patch, so that would be a separate issue from the one I am addressing in this review.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113142