[PATCH] D41743: Debug Info: Support DW_AT_calling_convention on composite types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/CodeGen/CGDebugInfo.cpp:2812-2814
+  // Implicitly by reference is everything clearly looks like a non-pod type:
+  //  - Types that define a destructor, a copy constructor, and a copy
+  //    assignment operator.
----------------
I do not believe this is sufficient. Consider:

```
struct A {
  A(const A&) = default;
  A(A&);
};
struct B : A {};
```

Now, `A` is passed indirectly, because it has a non-trivial copy constructor (`A::A(A&)`). But `B` is passed directly, because it only has a trivial copy constructor, despite having an `A` subobject.

I would not expect debuggers to get intricacies like this right. (In general, determining how to pass a class like `B` can require performing template instantiation, which it's clearly not reasonable to expect a debugger to do.)

If you want to give the debugger a hint for any type that's non-POD (as the comment says), then use `CXXRecordDecl::isPOD` to determine that. I think it is reasonable to believe that a debugger will figure out that objects of POD types are passed direct.


================
Comment at: lib/CodeGen/CGDebugInfo.cpp:2817
+  if (auto CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+    if (CXXRD->isTriviallyCopyable()) {
+      if (CXXRD->hasUserDeclaredDestructor() ||
----------------
"isTriviallyCopyable()" is the wrong thing to check here. The best you can do to get a per-type "calling convention" value is to call `CGCXXABI::getRecordArgABI`.

However, that does not tell the full story: in particular, in the MS ABI, member functions always return objects of class type indirectly, regardless of whether they could otherwise be returned in registers. If DWARF 5 allows you to override the calling convention on a particular usage of a type, you can check whether a function's return type should be indirect by calling `CGCXXABI::classifyReturnType`.


================
Comment at: lib/CodeGen/CGDebugInfo.cpp:2821-2822
+          CXXRD->hasUserDeclaredMoveConstructor() ||
+          CXXRD->hasUserDeclaredCopyAssignment() ||
+          CXXRD->hasUserDeclaredMoveAssignment())
+        Flags |= llvm::DINode::FlagTypePassByValue;
----------------
The assignment operators are irrelevant here. Does any debugger really base its calling convention decision on their existence?


================
Comment at: lib/CodeGen/CGDebugInfo.cpp:2825-2827
+      if (!CXXRD->hasUserDeclaredDestructor() &&
+          !CXXRD->hasUserDeclaredCopyConstructor() &&
+          !CXXRD->hasUserDeclaredCopyAssignment())
----------------
Why are you not checking for move operations on this side?


Repository:
  rL LLVM

https://reviews.llvm.org/D41743