[PATCH] D90892: [AIX][FE] Support constructor/destructor attribute

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 5 inline comments as done.
Xiangling_L added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.h:1482
+  void AddGlobalDtor(llvm::Function *Dtor, int Priority = 65535,
+                     bool IsDtorAttrFunc = false);
 
----------------
aaron.ballman wrote:
> Xiangling_L wrote:
> > aaron.ballman wrote:
> > > There's a fixme comment a few lines up about hardcoding priority being gross and this sort of extends the grossness a bit. Perhaps these functions should accept a `DestructorAttr *`/`ConstructorAttr *` that can be null?
> > Yeah, I can understand that putting a magic number as 65535 here is gross, but a `bool` with default argument also falls into that way? Or you are indicating it's better to not use default argument?
> I think the signature should be:
> ```
> void AddGlobalDtor(llvm::Function *Dtor, DestructorAttr *DA = nullptr);
> ```
> (I don't have strong opinions about whether the attribute pointer should be defaulted to null or not.) `IsDtorAttrFunc` is implied by a nonnull pointer and the priority can be gleaned directly from that attribute (or set to the magic number if the attribute pointer is null).
Oh, I see what do you mean here. But the issue is `AddGlobalDtor` is not only used for dtor attribute functions, so we cannot always glean the priority from a `DestructorAttr`.

If use `DestructorAttr`, the function signature has to be:


```
void AddGlobalDtor(llvm::Function *Dtor, int Priority, DestructorAttr *DA = nullptr);
```

So that's why I think a `bool` is good enough here.


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2584
+      // We're assuming that the destructor function is something we can
+      // reasonably call with the default CC.  Go ahead and cast it to the
+      // right prototype.
----------------
aaron.ballman wrote:
> Xiangling_L wrote:
> > aaron.ballman wrote:
> > > Is this assumption safe though given that there are calling convention attributes that can be written on the function?
> > Actually I copied this comment from where linux implemented the dtor attribute functions. I think it makes sense to make this assumption. Because when they are used as destructor functions, they actually don't have any caller from source.
> Ah, the comment confused me because the user could always write something like:
> ```
> [[clang::stdcall, gnu::destructor]] void func();
> ```
> where the destructor function is not something you can call with the default (cdecl) calling convention. Should the comment say "we can reasonably call with the correct CC" instead to avoid this confusion?
Sure, that would make more sense under AIX context.


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

https://reviews.llvm.org/D90892