Updated for comment from @jmorse

Updated to incorporate comments from @djtodoro .

In D84113#2390725, @alok wrote:

Updated comments from @aprantl and @dblaikie (on other review).

Updated to re-base.

Updated to re-base.

Updated to re-base.

Updated to re-base.

Updated to re-base and include many comments from @jmorse .

Updated comments from @aprantl and @dblaikie (on other review).

In D84114#2355465, @dblaikie wrote:

In D84114#2343689, @alok wrote:

In D84114#2336165, @dblaikie wrote:

Where's the name "explicit" pointer come from? I'm not sure how to read/understand the naming distinction between the names "implicit pointer" and "explicit pointer". Maybe the name should be "LLVM_implicit_pointer" as distinct from the DWARF "implicit_pointer" - that they're roughly the same concept, but the LLVM one is a generalization of it? Or is there something in the implicit/explicit distinction that helps clarify the semantic distinction between the two?

We are using three operators DW_OP_LLVM_explicit_pointer, DW_OP_LLVM_implicit_pointer, DW_OP_implicit_pointer.
Last operator DW_OP_implicit_pointer is dumped to object file and is exact syntax as in DWARF document (DW_OP_implicit_pointer <DIE_reference> <offset_in_DIE>)
Second operator DW_OP_LLVM_implicit_pointer is little variation of DW_OP_implicit_pointer for use till the time the DIE reference is not available with the different syntax (DW_OP_LLVM_implicit_pointer <offset>), where one argument is implicit (first argument of dbg.value intrinsic).

OK, I'm with you so far, that sounds reasonable to me.

While last two operators refer to another variable (Metadata in DW_OP_LLVM_implicit_pointer and DIE reference in DW_OP_implicit_pointer), So this is the variable where we can get the value pointed to by pointer variable (implicit way of getting the value).
The first operator DW_OP_LLVM_explicit_pointer is different than other two operators. This operator is to represent the constant value itself than the other variable and it has the direct way of getting the value pointed to by pointer variable, so the name "explicit" is used.

Sorry, I don't think I'm understanding "This operator is to represent the constant value itself than the other variable and it has the direct way of getting the value pointed to by pointer variable"

Oh, this is for a case where, say, I wrote code like this:

const int *f1() {
  static const int x = 3;
  return x;
}
void f2() {
  const int *y = f1();
  ...
}

Why does that need a different DW_OP compared to DW_OP_LLVM_implicit_pointer? Presumably we use dbg.values for variables with constant values without needing extra operators? (so a variable with the constant value 3 is call void @llvm.dbg.value(metadata i32 3, metadata !11, metadata !DIExpression()) and a variable that /points to/ the constant value 3 would, I would've thought, be call void @llvm.dbg.value(metadata i32 3, metadata !11, metadata !DIExpression(DW_OP_LLVM_implicit_pointer))

though I am open to use any other name.

Addressed comments from @aprantl to specialize DIBuilder routine arguments and added tests.

In D90243#2356751, @djtodoro wrote:

It was huge. Thanks.

Addressed concern of @aprantl by getting rid of ConstantAsMetadata and using DIExpression to represent integer type of bounds.

In D89817#2348364, @cchen15 wrote:

Typed the attributes as DIExpression*, as suggested by @aprantl.

In D84114#2336165, @dblaikie wrote:

Where's the name "explicit" pointer come from? I'm not sure how to read/understand the naming distinction between the names "implicit pointer" and "explicit pointer". Maybe the name should be "LLVM_implicit_pointer" as distinct from the DWARF "implicit_pointer" - that they're roughly the same concept, but the LLVM one is a generalization of it? Or is there something in the implicit/explicit distinction that helps clarify the semantic distinction between the two?

In D89218#2335920, @aprantl wrote:

Cab you remind me why we are allowing ConstantAsMetadata nodes at all instead of allowing only DIExpressions?

In D84114#2324425, @alok wrote:

In D84114#2291599, @dblaikie wrote:

hmm, possibly more words in the patch description (& IR documentation). of this and the previous patch - what is implicit_pointer V explicit_pointer? Maybe some examples in the patch description and IR documentation about how they could be used?

Thanks for your comment. I shall update the patch description and IR documentation as well.

Updated to re-base and incorporate comment from @dblaikie .

Incorporated comments from @djtodoro and also removed function re-naming due to pre-built clang-tidy warnings.
Now all other format warning except first are corrected.

Updated to re-base, incorporate comment from @SouraVX and changes to pacify pre-built check which expects function names to start with small letter and Variable with capital letter, Newer added function gets created with macro. Changed macro and consequently had to change existing functions.

In D89208#2325611, @aprantl wrote:

Can you add an assembler roundtrip test to test/Assembler to test the change to lib/IR?
It just can contain a single named node that holds a DIExpression.

Updated to re-base and incorporate comment from @aprantl .

In D84114#2291599, @dblaikie wrote:

hmm, possibly more words in the patch description (& IR documentation). of this and the previous patch - what is implicit_pointer V explicit_pointer? Maybe some examples in the patch description and IR documentation about how they could be used?

Updated with comment from @djtodoro

Re-based,

Thanks @aprantl for reviewing this. Any idea how to backport it and other fortran related commits to older version of LLVM (11, 10, 9) ?

Incorporated comment from @aprantl .

Updated to incorporate comment from @aprantl .

Updated to incorporate @aprantl 's comment.

In D87406#2266722, @rnk wrote:

I would ask you to add a test, but I can't recommend the existing test, llvm/test/DebugInfo/COFF/types-array.ll, as a good basis for a test. I'll look into rewriting it and adding coverage after you land.

@jmorse do you have any more comment on this ?

@jmorse Do you have any more comment on this ?

In D84114#2184522, @jmorse wrote:

Similar story to the parent patch, could you put a file-comment into the test explaining it's a round-trip test?

Incorporated comments from @jmorse .

Thanks for your reviewing this.

Incorporated comments from @aprantl .

In D83544#2151182, @aprantl wrote:

Mechanically, this looks great! it would be good to add a little more context to the LangRef documentation (see my inline comment).