[PATCH] D127625: [Debug] [Coroutines] Get rid of DW_ATE_address

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:925
     RetType = Builder.createBasicType(OS.str(), Layout.getTypeSizeInBits(Ty),
-                                      dwarf::DW_ATE_address,
-                                      llvm::DINode::FlagArtificial);
+      dwarf::DW_ATE_unsigned_char, llvm::DINode::FlagArtificial);
   }
----------------
How come this one became `unsigned char` rather than pointer type?


================
Comment at: llvm/test/Transforms/Coroutines/coro-debug-coro-frame.ll:16-17
 ;
+; CHECK-NOT: DW_ATE_address
+;
 ; CHECK-DAG: ![[FILE:[0-9]+]] = !DIFile(filename: "coro-debug.cpp"
----------------
CHECK-NOT only applies between one CHECK and the next, so this probably won't do much/not enough to carry its weight, I'd guess?

(--implicit-check-not as an argument to FileCheck anti-checks for the value everywhere, but even then - I'm happy to drop this testing if the source code no longer mentions DW_ATE_address anywhere anyway, and the things being emitted are being actively tested for instead)


================
Comment at: llvm/test/Transforms/Coroutines/coro-debug-coro-frame.ll:36
+; CHECK-DAG: ![[INT64_PTR]] = !DIDerivedType(tag: DW_TAG_member, name: "__int_64_Ptr_3",{{.*}} baseType: ![[INT64_PTR_BASE:[0-9]+]]
+; CHECK-DAG: ![[INT64_PTR_BASE]] = !DIDerivedType(tag: DW_TAG_pointer_type, name: "__int_64_Ptr", baseType: null, size: 64
+; CHECK-DAG: ![[INT32_2]] = !DIDerivedType(tag: DW_TAG_member, name: "__int_32_4", scope: ![[FRAME_TYPE]], file: ![[FILE]], line: [[PROMISE_VAR_LINE]], baseType: ![[I32_BASE:[0-9]+]]{{.*}}, flags: DIFlagArtificial
----------------
Why does this type have a name? I think it should probably be nameless - so it should merge with other instances of the same pointer type created for the original source code, etc.


================
Comment at: llvm/test/Transforms/Coroutines/coro-debug-coro-frame.ll:63-64
 declare void @consume(%struct.big_structure*)
+declare void @produce_vector(<4 x i32> *)
+declare void @consume_vector(<4 x i32> *)
 declare void @pi32(i32*)
----------------
why are there changes to the IR here? I guess they're necessary to create the interesting debug info IR being tested?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127625