[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

awpandey added inline comments.


================
Comment at: clang/test/CodeGenCXX/debug-info-template-align.cpp:8
+
+// CHECK: DIDerivedType(tag: DW_TAG_typedef, {{.*}}, align:
+
----------------
aprantl wrote:
> Do we need to hardcode the target here? Could we check for the specific align value?
Sorry, I could not get your first comment

> Do we need to hardcode the target here?

I have incorporated your rest of the suggestions.




================
Comment at: llvm/include/llvm/IR/DIBuilder.h:243
+                                 unsigned LineNo, DIScope *Context,
+                                 uint32_t AlignInBits = 0);
 
----------------
aprantl wrote:
> This should be `Optional<uint32_t>` AlignInBits. Even better perhaps `llvm::Align` but perhaps that's too restrictive.
Yes @aprantl  this should be of `Optional<unit_32t>`  type but changing this will require change in the `DIDerivedType::get` macro and this macro is used by many other functions. Please correct me if I am wrong. ??

Current functionality of the `createTypeDef` is like the default value of the `alignInBits`  will be `0`. Consider below comment in `DIBuilder.cpp`.


================
Comment at: llvm/lib/IR/DIBuilder.cpp:309
-                                        DIScope *Context) {
+                                        DIScope *Context,
+                                        uint32_t AlignInBits) {
   return DIDerivedType::get(VMContext, dwarf::DW_TAG_typedef, Name, File,
-                            LineNo, getNonCompileUnitScope(Context), Ty, 0, 0,
----------------
Consider the 9th argument here 
```
return DIDerivedType::get(VMContext, dwarf::DW_TAG_typedef, Name, File,LineNo, getNonCompileUnitScope(Context), Ty, 0, 0, --Hard coded alignment for the typedef field in existing API.
0, None, DINode::FlagZero);
}
```
 That is why rather than changing the entire API by using `Option<uint_32t>` I used `uint_32`


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

https://reviews.llvm.org/D70111