[DebugInfo] Expose Fortran array debug info attributes through DIBuilder.

[cchen15]

Summary

The support of a few debug info attributes specifically for Fortran arrays have been added to LLVM recently, but there's no way to take advantage of them through DIBuilder. This patch extends DIBuilder::createArrayType to enable the settings of those attributes.

[cchen15]

I'd appreciate some guidance on what test to add for this change. Would it be sufficient to have a unittest that calls createArrayType to create a DICompositeType, and then get the three new attributes back from the DICompositeType using the getRaw* functions to check that they match with the arguments to the createArrayType call? Thanks.

[aprantl]

Can you also add a unit test for this?

llvm/include/llvm/IR/DIBuilder.h

497

nit: . at the end please

498

Please also document what kind of DINode is expected here.

507

Can these be any more specific DINode types?

[cchen15]

Regarding test: Does the test idea laid out in my opening comment sound reasonable?

llvm/include/llvm/IR/DIBuilder.h

507

These can be DIExpression, ConstantAsMetadata, etc, so Metadata looks to be the right specificity (and the underlying 'get' function it calls types them as Metadata too).

[cchen15]

Per review comments, fixed/enhanced the description of createArrayType.

[cchen15]

Per review comments, fixed/enhanced the description of createTypeArray.

[cchen15]

Sorry. Please ignore Patch 2.

[cchen15]

Added a unit test for the patch.

[alok]

llvm/lib/IR/DIBuilder.cpp

533

You may want to add "Rank" as well. (D89141)

[cchen15]

Including the rank attribute in the patch.

[SouraVX]

Thanks for the patch and adding the unit-test for the same. This API is still un-used(directly) (classic flang uses it's own way to construct this.).
The new Flang is the planned consumer for it, Although since most CodeGen part of new Flang is still lives downstream. There's not much we can do about it WRT to debug-info and these API utilization.

LGTM! I'll leave to @aprantl for official approval(since he's senior member and understands the API interfacing and debug metadata much better) :).

[aprantl]

These can be DIExpression, ConstantAsMetadata, etc, so Metadata looks to be the right specificity (and the underlying 'get' function it calls types them as Metadata too).

Why do we need to accept both DIExpression and ConstantAsMetadata? Since we can also represent all constants as DIExpression, I think I would prefer to have only one way to represent a constant. Would it be feasible to expect clients to pass constants as DIExpression(DW_OP_constu, <number>, DW_OP_stack_value)? And if it doesn't already exist perhaps add a DIExpression *DIBuilder::createConstantExpression(unsigned value) API?

[cchen15]

In D89817#2347933, @aprantl wrote:

These can be DIExpression, ConstantAsMetadata, etc, so Metadata looks to be the right specificity (and the underlying 'get' function it calls types them as Metadata too).

Why do we need to accept both DIExpression and ConstantAsMetadata? Since we can also represent all constants as DIExpression, I think I would prefer to have only one way to represent a constant. Would it be feasible to expect clients to pass constants as DIExpression(DW_OP_constu, <number>, DW_OP_stack_value)? And if it doesn't already exist perhaps add a DIExpression *DIBuilder::createConstantExpression(unsigned value) API?

We can do that, but then the debugger would need to work a bit harder in order to get a constant, doesn't it? I don't feel strongly either way. If you do want it done the way you prefer, please let me know and I will make the change.

[cchen15]

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

[cchen15]

Also, DIBuilder::createConstantValueExpression(uint64_t) is already available for doing constant via DIExpression,, and its usage is demonstrated in the unit test.

[alok]

In D89817#2348364, @cchen15 wrote:

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

Sorry for late comment. But We would miss the DIVariable type in that case which is also permissible for attributes, which later (in DWARF) is converted to DIE reference.

[cchen15]

Reverted back to type those attributes as Metadata to allow DIVairbale.

Thanks, @alok, for pointing out the DIVariable case.

[aprantl]

We can do that, but then the debugger would need to work a bit harder in order to get a constant, doesn't it? I don't feel strongly either way. If you do want it done the way you prefer, please let me know and I will make the change.

That's an interesting point. Do we emit different DWARF FORMs for constant vs constant DIExpression? An elegant solution would be to recognize constant DIExpressions in AsmPrinter (DIExpression::isConstant) and emit the same DWARF FORM we would emit for an IR constant if the expression is a constant.

[aprantl]

llvm/include/llvm/IR/DIBuilder.h

508

We could make this still strongly typed by either having two variants of the function or by using a PointerUnion<DIExpression, DIVariable> here.

[cchen15]

In D89817#2353822, @aprantl wrote:

We can do that, but then the debugger would need to work a bit harder in order to get a constant, doesn't it? I don't feel strongly either way. If you do want it done the way you prefer, please let me know and I will make the change.

That's an interesting point. Do we emit different DWARF FORMs for constant vs constant DIExpression? An elegant solution would be to recognize constant DIExpressions in AsmPrinter (DIExpression::isConstant) and emit the same DWARF FORM we would emit for an IR constant if the expression is a constant.

This optimization is orthogonal to the change proposed here, IMHO. I'd prefer keep this patch to DIBuilder, and do that optimization in a separate patch. Would that be ok with you?

[cchen15]

Typed the attributes as PointerUnion<DIExpression *, DIVariable *>, and updated the test to exercise the DIVariable case.

[cchen15]

llvm/include/llvm/IR/DIBuilder.h

508

Thanks for pointing this out.

[hctim]

This patch unfortunately caused a regression in the sanitizer buildbots, namely a check-llvm regression under ASan. Please take a look:

http://lab.llvm.org:8011/#/builders/99/builds/253

********************
Testing:  0..
FAIL: LLVM-Unit :: IR/./IRTests/DIBuilder.CreateFortranArrayTypeWithAttributes (3304 of 39961)
******************** TEST 'LLVM-Unit :: IR/./IRTests/DIBuilder.CreateFortranArrayTypeWithAttributes' FAILED ********************
Note: Google Test filter = DIBuilder.CreateFortranArrayTypeWithAttributes
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from DIBuilder
[ RUN      ] DIBuilder.CreateFortranArrayTypeWithAttributes
[       OK ] DIBuilder.CreateFortranArrayTypeWithAttributes (1 ms)
[----------] 1 test from DIBuilder (1 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (2 ms total)
[  PASSED  ] 1 test.
=================================================================
==1460==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 104 byte(s) in 1 object(s) allocated from:
    #0 0x6975b8 in operator new(unsigned long) /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:99:3
    #1 0x1990b21 in llvm::MDNode::operator new(unsigned long, unsigned int) /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/lib/IR/Metadata.cpp:485:40
    #2 0x17b3aa3 in llvm::DIGlobalVariable::getImpl(llvm::LLVMContext&, llvm::Metadata*, llvm::MDString*, llvm::MDString*, llvm::Metadata*, unsigned int, llvm::Metadata*, bool, bool, llvm::Metadata*, llvm::Metadata*, unsigned int, llvm::Metadata::StorageType, bool) /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/lib/IR/DebugInfoMetadata.cpp:902:3
    #3 0x17736e8 in getImpl /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:2827:12
    #4 0x17736e8 in getTemporary /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:2848:3
    #5 0x17736e8 in llvm::DIBuilder::createTempGlobalVariableFwdDecl(llvm::DIScope*, llvm::StringRef, llvm::StringRef, llvm::DIFile*, unsigned int, llvm::DIType*, bool, llvm::MDNode*, llvm::MDTuple*, unsigned int) /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/lib/IR/DIBuilder.cpp:697:10
    #6 0x8723f4 in (anonymous namespace)::DIBuilder_CreateFortranArrayTypeWithAttributes_Test::TestBody() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/unittests/IR/DebugInfoTest.cpp:213:11
    #7 0x256d6c0 in HandleExceptionsInMethodIfSupported<testing::Test, void> /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc
    #8 0x256d6c0 in testing::Test::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2474:5
    #9 0x256fcd5 in testing::TestInfo::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2656:11
    #10 0x25710d7 in testing::TestCase::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2774:28
    #11 0x258f30d in testing::internal::UnitTestImpl::RunAllTests() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:4649:43
    #12 0x258e5c0 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc
    #13 0x258e5c0 in testing::UnitTest::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:4257:10
    #14 0x25530e0 in RUN_ALL_TESTS /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46
    #15 0x25530e0 in main /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50:10
    #16 0x7f9c9071209a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
Indirect leak of 128 byte(s) in 1 object(s) allocated from:
    #0 0x6975b8 in operator new(unsigned long) /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:99:3
    #1 0x198df3e in make_unique<llvm::ReplaceableMetadataImpl, llvm::LLVMContext &> /b/sanitizer-x86_64-linux-bootstrap/build/libcxx_build_asan/include/c++/v1/memory:2816:28
    #2 0x198df3e in llvm::ContextAndReplaceableUses::getOrCreateReplaceableUses() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/include/llvm/IR/Metadata.h:816:23
    #3 0x198a533 in getOrCreate /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/lib/IR/Metadata.cpp:315:51
    #4 0x198a533 in llvm::MetadataTracking::track(void*, llvm::Metadata&, llvm::PointerUnion<llvm::MetadataAsValue*, llvm::Metadata*>) /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/lib/IR/Metadata.cpp:157:17
    #5 0x19955d3 in track /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/include/llvm/IR/Metadata.h
    #6 0x19955d3 in llvm::MDOperand::reset(llvm::Metadata*, llvm::Metadata*) /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/include/llvm/IR/Metadata.h:740:5
    #7 0x19910f2 in setOperand /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/lib/IR/Metadata.cpp:874:22
    #8 0x19910f2 in llvm::MDNode::MDNode(llvm::LLVMContext&, unsigned int, llvm::Metadata::StorageType, llvm::ArrayRef<llvm::Metadata*>, llvm::ArrayRef<llvm::Metadata*>) /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/lib/IR/Metadata.cpp:511:5
    #9 0x17a8fe5 in DINode /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:125:9
    #10 0x17a8fe5 in DIScope /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:426:9
    #11 0x17a8fe5 in DIType /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:619:9
    #12 0x17a8fe5 in DICompositeType /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:1007:9
    #13 0x17a8fe5 in llvm::DICompositeType::getImpl(llvm::LLVMContext&, unsigned int, llvm::MDString*, llvm::Metadata*, unsigned int, llvm::Metadata*, llvm::Metadata*, unsigned long, unsigned int, unsigned long, llvm::DINode::DIFlags, llvm::Metadata*, unsigned int, llvm::Metadata*, llvm::Metadata*, llvm::MDString*, llvm::Metadata*, llvm::Metadata*, llvm::Metadata*, llvm::Metadata*, llvm::Metadata*, llvm::Metadata::StorageType, bool) /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/lib/IR/DebugInfoMetadata.cpp:524:3
    #14 0x177290b in getImpl /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:1031:12
    #15 0x177290b in get /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:1059:3
    #16 0x177290b in llvm::DIBuilder::createArrayType(unsigned long, unsigned int, llvm::DIType*, llvm::MDTupleTypedArrayWrapper<llvm::DINode>, llvm::PointerUnion<llvm::DIExpression*, llvm::DIVariable*>, llvm::PointerUnion<llvm::DIExpression*, llvm::DIVariable*>, llvm::PointerUnion<llvm::DIExpression*, llvm::DIVariable*>, llvm::PointerUnion<llvm::DIExpression*, llvm::DIVariable*>) /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/lib/IR/DIBuilder.cpp:534:13
    #17 0x872979 in (anonymous namespace)::DIBuilder_CreateFortranArrayTypeWithAttributes_Test::TestBody() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/unittests/IR/DebugInfoTest.cpp:218:36
    #18 0x256d6c0 in HandleExceptionsInMethodIfSupported<testing::Test, void> /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc
    #19 0x256d6c0 in testing::Test::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2474:5
    #20 0x256fcd5 in testing::TestInfo::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2656:11
    #21 0x25710d7 in testing::TestCase::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2774:28
    #22 0x258f30d in testing::internal::UnitTestImpl::RunAllTests() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:4649:43
    #23 0x258e5c0 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc
    #24 0x258e5c0 in testing::UnitTest::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:4257:10
    #25 0x25530e0 in RUN_ALL_TESTS /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46
    #26 0x25530e0 in main /b/sanitizer-x86_64-linux-bootstrap/build/llvm-project/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50:10
    #27 0x7f9c9071209a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
SUMMARY: AddressSanitizer: 232 byte(s) leaked in 2 allocation(s).

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
********************
Failed Tests (3):
  ...
  LLVM-Unit :: IR/./IRTests/DIBuilder.CreateFortranArrayTypeWithAttributes

Note that there are two other tests that failed previously, from https://reviews.llvm.org/D89486#2358100.

llvm/include/llvm/IR/DIBuilder.h

DITemplateValueParameter *createTemplateParameterPack(DIScope *Scope,
                                                           DIType *Ty,
                                                           DINodeArray Val);
 
     /// Create debugging information entry for an array.
     /// \param Size         Array size.
     /// \param AlignInBits  Alignment.
     /// \param Ty           Element type.
     /// \param Subscripts   Subscripts.
+    /// \param DataLocation The location of the raw data of a descriptor-based
+    ///                     Fortran array.
+    /// \param Associated   The associated attribute of a descriptor-based
+    ///                     Fortran array.
+    /// \param Allocated    The allocated attribute of a descriptor-based
+    ///                     Fortran array.
+    /// \param Rank         The rank attribute of a descriptor-based
+    ///                     Fortran array.
     DICompositeType *createArrayType(uint64_t Size, uint32_t AlignInBits,
-                                     DIType *Ty, DINodeArray Subscripts);
+                                     DIType *Ty, DINodeArray Subscripts,
+                                     DIExpression *DataLocation = nullptr,
+                                     DIExpression *Associated = nullptr,
+                                     DIExpression *Allocated = nullptr,
+                                     DIExpression *Rank = nullptr);
 
     /// Create debugging information entry for a vector type.
     /// \param Size         Array size.
     /// \param AlignInBits  Alignment.
     /// \param Ty           Element type.
     /// \param Subscripts   Subscripts.
     DICompositeType *createVectorType(uint64_t Size, uint32_t AlignInBits,
                                       DIType *Ty, DINodeArray Subscripts);

llvm/lib/IR/DIBuilder.cpp

auto *CTy = DICompositeType::get(
       nullptr, UniqueIdentifier);
   AllEnumTypes.push_back(CTy);
   trackIfUnresolved(CTy);
   return CTy;
 }
 
 DICompositeType *DIBuilder::createArrayType(uint64_t Size,
                                             uint32_t AlignInBits, DIType *Ty,
-                                            DINodeArray Subscripts) {
+                                            DINodeArray Subscripts,
+                                            DIExpression *DataLocation,
+                                            DIExpression *Associated,
+                                            DIExpression *Allocated,
+                                            DIExpression *Rank) {
   auto *R = DICompositeType::get(VMContext, dwarf::DW_TAG_array_type, "",
                                  nullptr, 0, nullptr, Ty, Size, AlignInBits, 0,
-                                 DINode::FlagZero, Subscripts, 0, nullptr);
+                                 DINode::FlagZero, Subscripts, 0, nullptr,
+                                 nullptr, "", nullptr, DataLocation,
+                                 Associated, Allocated, Rank);
   trackIfUnresolved(R);
   return R;
 }
 
 DICompositeType *DIBuilder::createVectorType(uint64_t Size,
                                              uint32_t AlignInBits, DIType *Ty,
                                              DINodeArray Subscripts) {
   auto *R = DICompositeType::get(VMContext, dwarf::DW_TAG_array_type, "",

llvm/unittests/IR/DebugInfoTest.cpp

 //===- llvm/unittest/IR/DebugInfo.cpp - DebugInfo tests -------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/DebugInfo.h"
+#include "llvm/IR/DIBuilder.h"
 #include "llvm/AsmParser/Parser.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Transforms/Utils/Local.h"
)");
   SmallVector<DbgValueInst *, 1> DVIs;
   findDbgValues(DVIs, &I);
 
   // Delete %b. The dbg.value should now point to undef.
   I.eraseFromParent();
   EXPECT_TRUE(isa<UndefValue>(DVIs[0]->getValue()));
 }
 
+TEST(DIBuilder, CreateFortranArrayTypeWithAttributes) {
+  LLVMContext Ctx;
+  std::unique_ptr<Module> M(new Module("MyModule", Ctx));
+  DIBuilder DIB(*M);
+
+  DISubrange *Subrange = DIB.getOrCreateSubrange(1,1);
+  SmallVector<Metadata*, 4> Subranges;
+  Subranges.push_back(Subrange);
+  DINodeArray Subscripts = DIB.getOrCreateArray(Subranges);
+
+  auto getDIExpression = [&DIB](int offset) {
+    SmallVector<uint64_t, 4> ops;
+    ops.push_back(llvm::dwarf::DW_OP_push_object_address);
+    DIExpression::appendOffset(ops, offset);
+    ops.push_back(llvm::dwarf::DW_OP_deref);
+
+    return DIB.createExpression(ops);
+  };
+
+  DIExpression *DataLocation = getDIExpression(0);
+  DIExpression *Associated = getDIExpression(1);
+  DIExpression *Allocated = getDIExpression(2);
+  DIExpression *Rank = DIB.createConstantValueExpression(3);
+
+  DICompositeType *ArrayType = DIB.createArrayType(0, 0, nullptr, Subscripts,
+                                                   DataLocation, Associated,
+                                                   Allocated, Rank);
+
+  EXPECT_TRUE(isa_and_nonnull<DICompositeType>(ArrayType));
+  EXPECT_EQ(ArrayType->getRawDataLocation(), DataLocation);
+  EXPECT_EQ(ArrayType->getRawAssociated(), Associated);
+  EXPECT_EQ(ArrayType->getRawAllocated(), Allocated);
+  EXPECT_EQ(ArrayType->getRawRank(), Rank);
+}
+
 } // end namespace