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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: include/clang/Basic/TargetInfo.h:52
 
-/// Exposes information about the current target.
-///
-class TargetInfo : public RefCountedBase<TargetInfo> {
-  std::shared_ptr<TargetOptions> TargetOpts;
-  llvm::Triple Triple;
-protected:
-  // Target values set by the ctor of the actual target implementation.  Default
-  // values are specified by the TargetInfo constructor.
-  bool BigEndian;
-  bool TLSSupported;
-  bool VLASupported;
-  bool NoAsmVariants;  // True if {|} are normal characters.
-  bool HasLegalHalfType; // True if the backend supports operations on the half
-                         // LLVM IR type.
-  bool HasFloat128;
+/// Flags controlling how a type is layed out in memory.
+struct TransferrableTargetInfo {
----------------
"Fields controlling how types are laid out in memory; these may need to be copied for targets like AMDGPU that base their ABIs on an auxiliary CPU target."


================
Comment at: include/clang/Basic/TargetInfo.h:194
 
+  bool ShouldCopyAuxTarget;
+
----------------
Why is this flag necessary?  Can't `setAuxTarget` just decide whether or not to copy?

Specifically, I would suggest:
- Make `copyAuxTarget` be a non-virtual `protected` method that unconditionally copies the target.
- Make `setAuxTarget` a virtual method that does nothing by default.
- Override `setAuxTarget` for AMDGPU and call `copyAuxTarget`.


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

https://reviews.llvm.org/D56318