[PATCH] D56318: [HIP] Fix size_t for MSVC environment

Yaxun Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 10 13:03:20 PST 2019


yaxunl added a comment.

In D56318#1353176 <https://reviews.llvm.org/D56318#1353176>, @rjmccall wrote:

> In D56318#1353116 <https://reviews.llvm.org/D56318#1353116>, @yaxunl wrote:
>
> > In D56318#1353106 <https://reviews.llvm.org/D56318#1353106>, @rjmccall wrote:
> >
> > > No, I understand that things like the function-call ABI should be different from the associated host ABI, but things like the size of `long` and the bit-field layout algorithm presumably shouldn't be, and that's the sort of thing that's configured by `TargetInfo`.
> >
> >
> > How about create a ForwardingTargegInfo which will has a pointer to AuxTarget and forward to that target if it is not null. Then let AMDGPUTargetInfo inherit from that.
>
>
> Why forward?  You have, like, two supported host environments, right?  Can you just a subclass apiece of either `MicrosoftX86_64TargetInfo` or `X86_64TargetInfo`?
>
> If that's unreasonable and you do need to forward, having a `ForwardingTargetInfo` sounds like a good idea, although I think you should require it to have an underlying target, and I think you need it to copy all the fields of that target.


There are lots of child class of X86_64TargetInfo, e.g., CygwinX86_64TargetInfo, MicrosoftX86_64TargetInfo, MinGWX86_64TargetInfo, etc. to inherit each one of them will result in duplicated code. Also, many stuff in these TargetInfo do not apply to AMDGPU target. I think I should only selectively copy the relevant fields.


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

https://reviews.llvm.org/D56318





More information about the cfe-commits mailing list