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

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC352620: [HIP] Fix size_t for MSVC environment (authored by 
yaxunl, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56318?vs=184245=184274#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D56318

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets/AMDGPU.cpp
  lib/Basic/Targets/AMDGPU.h
  lib/Frontend/CompilerInstance.cpp
  test/SemaCUDA/amdgpu-size_t.cu

Index: lib/Basic/Targets/AMDGPU.cpp
===
--- lib/Basic/Targets/AMDGPU.cpp
+++ lib/Basic/Targets/AMDGPU.cpp
@@ -305,3 +305,7 @@
   if (hasFastFMA())
 Builder.defineMacro("FP_FAST_FMA");
 }
+
+void AMDGPUTargetInfo::setAuxTarget(const TargetInfo *Aux) {
+  copyAuxTarget(Aux);
+}
Index: lib/Basic/Targets/AMDGPU.h
===
--- lib/Basic/Targets/AMDGPU.h
+++ lib/Basic/Targets/AMDGPU.h
@@ -351,6 +351,8 @@
   uint64_t getNullPointerValue(LangAS AS) const override {
 return AS == LangAS::opencl_local ? ~0 : 0;
   }
+
+  void setAuxTarget(const TargetInfo *Aux) override;
 };
 
 } // namespace targets
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -796,3 +796,9 @@
   assert(getAccumIBits() >= getUnsignedAccumIBits());
   assert(getLongAccumIBits() >= getUnsignedLongAccumIBits());
 }
+
+void TargetInfo::copyAuxTarget(const TargetInfo *Aux) {
+  auto *Target = static_cast(this);
+  auto *Src = static_cast(Aux);
+  *Target = *Src;
+}
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -928,6 +928,9 @@
   // Adjust target options based on codegen options.
   getTarget().adjustTargetOptions(getCodeGenOpts(), getTargetOpts());
 
+  if (auto *Aux = getAuxTarget())
+getTarget().setAuxTarget(Aux);
+
   // rewriter project will change target built-in bool type from its default.
   if (getFrontendOpts().ProgramAction == frontend::RewriteObjC)
 getTarget().noSignedCharForObjCBool();
Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -48,22 +48,10 @@
 
 namespace Builtin { struct Info; }
 
-/// Exposes information about the current target.
-///
-class TargetInfo : public RefCountedBase {
-  std::shared_ptr 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;
-  bool HasFloat16;
+/// 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.
+struct TransferrableTargetInfo {
   unsigned char PointerWidth, PointerAlign;
   unsigned char BoolWidth, BoolAlign;
   unsigned char IntWidth, IntAlign;
@@ -104,15 +92,92 @@
   unsigned char SuitableAlign;
   unsigned char DefaultAlignForAttributeAligned;
   unsigned char MinGlobalAlign;
-  unsigned char MaxAtomicPromoteWidth, MaxAtomicInlineWidth;
+
+  unsigned short NewAlign;
   unsigned short MaxVectorAlign;
   unsigned short MaxTLSAlign;
+
+  const llvm::fltSemantics *HalfFormat, *FloatFormat, *DoubleFormat,
+*LongDoubleFormat, *Float128Format;
+
+  ///=== Target Data Type Query Methods ---===//
+  enum IntType {
+NoInt = 0,
+SignedChar,
+UnsignedChar,
+SignedShort,
+UnsignedShort,
+SignedInt,
+UnsignedInt,
+SignedLong,
+UnsignedLong,
+SignedLongLong,
+UnsignedLongLong
+  };
+
+  enum RealType {
+NoFloat = 255,
+Float = 0,
+Double,
+LongDouble,
+Float128
+  };
+protected:
+  IntType SizeType, IntMaxType, PtrDiffType, IntPtrType, WCharType,
+  WIntType, Char16Type, Char32Type, Int64Type, SigAtomicType,
+  ProcessIDType;
+
+  /// Whether Objective-C's built-in boolean type should be signed char.
+  ///
+  /// Otherwise, when this flag is not set, the normal built-in boolean type is
+  /// used.
+  unsigned UseSignedCharForObjCBool : 1;
+
+  /// Control whether the alignment of bit-field types is respected when laying
+  /// out structures. If true, then the alignment of the bit-field type will be
+  /// used to (a) impact the alignment of the containing structure, and (b)
+  /// ensure that 

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

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 184245.
yaxunl added a comment.

Use const argument.


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

https://reviews.llvm.org/D56318

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets/AMDGPU.cpp
  lib/Basic/Targets/AMDGPU.h
  lib/Frontend/CompilerInstance.cpp
  test/SemaCUDA/amdgpu-size_t.cu

Index: test/SemaCUDA/amdgpu-size_t.cu
===
--- /dev/null
+++ test/SemaCUDA/amdgpu-size_t.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple x86_64-pc-windows-msvc -fms-compatibility -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+typedef unsigned __int64 size_t;
+typedef __int64 intptr_t;
+typedef unsigned __int64 uintptr_t;
+
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -928,6 +928,9 @@
   // Adjust target options based on codegen options.
   getTarget().adjustTargetOptions(getCodeGenOpts(), getTargetOpts());
 
+  if (auto *Aux = getAuxTarget())
+getTarget().setAuxTarget(Aux);
+
   // rewriter project will change target built-in bool type from its default.
   if (getFrontendOpts().ProgramAction == frontend::RewriteObjC)
 getTarget().noSignedCharForObjCBool();
Index: lib/Basic/Targets/AMDGPU.h
===
--- lib/Basic/Targets/AMDGPU.h
+++ lib/Basic/Targets/AMDGPU.h
@@ -351,6 +351,8 @@
   uint64_t getNullPointerValue(LangAS AS) const override {
 return AS == LangAS::opencl_local ? ~0 : 0;
   }
+
+  void setAuxTarget(const TargetInfo *Aux) override;
 };
 
 } // namespace targets
Index: lib/Basic/Targets/AMDGPU.cpp
===
--- lib/Basic/Targets/AMDGPU.cpp
+++ lib/Basic/Targets/AMDGPU.cpp
@@ -305,3 +305,7 @@
   if (hasFastFMA())
 Builder.defineMacro("FP_FAST_FMA");
 }
+
+void AMDGPUTargetInfo::setAuxTarget(const TargetInfo *Aux) {
+  copyAuxTarget(Aux);
+}
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -796,3 +796,9 @@
   assert(getAccumIBits() >= getUnsignedAccumIBits());
   assert(getLongAccumIBits() >= getUnsignedLongAccumIBits());
 }
+
+void TargetInfo::copyAuxTarget(const TargetInfo *Aux) {
+  auto *Target = static_cast(this);
+  auto *Src = static_cast(Aux);
+  *Target = *Src;
+}
Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -48,22 +48,10 @@
 
 namespace Builtin { struct Info; }
 
-/// Exposes information about the current target.
-///
-class TargetInfo : public RefCountedBase {
-  std::shared_ptr 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;
-  bool HasFloat16;
+/// 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.
+struct TransferrableTargetInfo {
   unsigned char PointerWidth, PointerAlign;
   unsigned char BoolWidth, BoolAlign;
   unsigned char IntWidth, IntAlign;
@@ -104,15 +92,92 @@
   unsigned char SuitableAlign;
   unsigned char DefaultAlignForAttributeAligned;
   unsigned char MinGlobalAlign;
-  unsigned char MaxAtomicPromoteWidth, MaxAtomicInlineWidth;
+
+  unsigned short NewAlign;
   unsigned short MaxVectorAlign;
   unsigned short MaxTLSAlign;
+
+  const llvm::fltSemantics *HalfFormat, *FloatFormat, *DoubleFormat,
+*LongDoubleFormat, *Float128Format;
+
+  ///=== Target Data Type Query Methods ---===//
+  enum IntType {
+NoInt = 0,
+SignedChar,
+UnsignedChar,
+SignedShort,
+UnsignedShort,
+SignedInt,
+UnsignedInt,
+SignedLong,
+UnsignedLong,
+SignedLongLong,
+UnsignedLongLong
+  };
+
+  enum RealType {
+NoFloat = 255,
+Float = 0,
+Double,
+LongDouble,
+Float128
+  };
+protected:
+  IntType SizeType, IntMaxType, PtrDiffType, IntPtrType, WCharType,
+  WIntType, Char16Type, Char32Type, Int64Type, SigAtomicType,
+  ProcessIDType;
+
+  /// Whether Objective-C's built-in boolean type should be signed char.
+  ///
+  /// Otherwise, when this flag is not set, the normal built-in boolean type is
+  /// used.
+  unsigned UseSignedCharForObjCBool : 1;
+
+  /// 

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

One minor change and then LGTM.




Comment at: include/clang/Basic/TargetInfo.h:1352
+  /// Copy type and layout related info.
+  void copyAuxTarget(TargetInfo *Aux);
   virtual uint64_t getPointerWidthV(unsigned AddrSpace) const {

This can take a `const TargetInfo *`, which also very clearly documents 
expectations.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 184238.
yaxunl added a comment.

Revised by John's comments.


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

https://reviews.llvm.org/D56318

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets/AMDGPU.cpp
  lib/Basic/Targets/AMDGPU.h
  lib/Frontend/CompilerInstance.cpp
  test/SemaCUDA/amdgpu-size_t.cu

Index: test/SemaCUDA/amdgpu-size_t.cu
===
--- /dev/null
+++ test/SemaCUDA/amdgpu-size_t.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple x86_64-pc-windows-msvc -fms-compatibility -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+typedef unsigned __int64 size_t;
+typedef __int64 intptr_t;
+typedef unsigned __int64 uintptr_t;
+
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -928,6 +928,9 @@
   // Adjust target options based on codegen options.
   getTarget().adjustTargetOptions(getCodeGenOpts(), getTargetOpts());
 
+  if (auto *Aux = getAuxTarget())
+getTarget().setAuxTarget(Aux);
+
   // rewriter project will change target built-in bool type from its default.
   if (getFrontendOpts().ProgramAction == frontend::RewriteObjC)
 getTarget().noSignedCharForObjCBool();
Index: lib/Basic/Targets/AMDGPU.h
===
--- lib/Basic/Targets/AMDGPU.h
+++ lib/Basic/Targets/AMDGPU.h
@@ -351,6 +351,8 @@
   uint64_t getNullPointerValue(LangAS AS) const override {
 return AS == LangAS::opencl_local ? ~0 : 0;
   }
+
+  void setAuxTarget(TargetInfo *Aux) override;
 };
 
 } // namespace targets
Index: lib/Basic/Targets/AMDGPU.cpp
===
--- lib/Basic/Targets/AMDGPU.cpp
+++ lib/Basic/Targets/AMDGPU.cpp
@@ -305,3 +305,5 @@
   if (hasFastFMA())
 Builder.defineMacro("FP_FAST_FMA");
 }
+
+void AMDGPUTargetInfo::setAuxTarget(TargetInfo *Aux) { copyAuxTarget(Aux); }
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -796,3 +796,9 @@
   assert(getAccumIBits() >= getUnsignedAccumIBits());
   assert(getLongAccumIBits() >= getUnsignedLongAccumIBits());
 }
+
+void TargetInfo::copyAuxTarget(TargetInfo *Aux) {
+  auto *Target = static_cast(this);
+  auto *Src = static_cast(Aux);
+  *Target = *Src;
+}
Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -48,22 +48,10 @@
 
 namespace Builtin { struct Info; }
 
-/// Exposes information about the current target.
-///
-class TargetInfo : public RefCountedBase {
-  std::shared_ptr 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;
-  bool HasFloat16;
+/// 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.
+struct TransferrableTargetInfo {
   unsigned char PointerWidth, PointerAlign;
   unsigned char BoolWidth, BoolAlign;
   unsigned char IntWidth, IntAlign;
@@ -104,15 +92,92 @@
   unsigned char SuitableAlign;
   unsigned char DefaultAlignForAttributeAligned;
   unsigned char MinGlobalAlign;
-  unsigned char MaxAtomicPromoteWidth, MaxAtomicInlineWidth;
+
+  unsigned short NewAlign;
   unsigned short MaxVectorAlign;
   unsigned short MaxTLSAlign;
+
+  const llvm::fltSemantics *HalfFormat, *FloatFormat, *DoubleFormat,
+*LongDoubleFormat, *Float128Format;
+
+  ///=== Target Data Type Query Methods ---===//
+  enum IntType {
+NoInt = 0,
+SignedChar,
+UnsignedChar,
+SignedShort,
+UnsignedShort,
+SignedInt,
+UnsignedInt,
+SignedLong,
+UnsignedLong,
+SignedLongLong,
+UnsignedLongLong
+  };
+
+  enum RealType {
+NoFloat = 255,
+Float = 0,
+Double,
+LongDouble,
+Float128
+  };
+protected:
+  IntType SizeType, IntMaxType, PtrDiffType, IntPtrType, WCharType,
+  WIntType, Char16Type, Char32Type, Int64Type, SigAtomicType,
+  ProcessIDType;
+
+  /// Whether Objective-C's built-in boolean type should be signed char.
+  ///
+  /// Otherwise, when this flag is not set, the normal built-in boolean type is
+  /// used.
+  unsigned UseSignedCharForObjCBool : 1;
+
+  /// Control 

[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 {
-  std::shared_ptr 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D56318#1355705 , @rjmccall wrote:

> It's pretty unfortunate that all these fields have to be individually called 
> out like this.  Can you move all these basic layout fields into a separate 
> `struct` (which can be a secondary base class of `TargetInfo`) which can then 
> just be normally copied?  Anything that needs special copy semantics, like 
> the LLVM `DataLayout` (do you need to copy this?) doesn't need to go into 
> that struct, just the basic POD things that determine fundamental type 
> layouts and semantics.


LLVM DataLayout contains target specific stuff and cannot be simply copied. So 
far we did not see necessity to adjust device data layout for host.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 182815.
yaxunl added a comment.

separate layout controlling flags to a base class for TargetInfo.


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

https://reviews.llvm.org/D56318

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets/AMDGPU.cpp
  lib/Frontend/CompilerInstance.cpp
  test/SemaCUDA/amdgpu-size_t.cu

Index: test/SemaCUDA/amdgpu-size_t.cu
===
--- /dev/null
+++ test/SemaCUDA/amdgpu-size_t.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple x86_64-pc-windows-msvc -fms-compatibility -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+typedef unsigned __int64 size_t;
+typedef __int64 intptr_t;
+typedef unsigned __int64 uintptr_t;
+
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -929,6 +929,9 @@
   // Adjust target options based on codegen options.
   getTarget().adjustTargetOptions(getCodeGenOpts(), getTargetOpts());
 
+  if (auto *Aux = getAuxTarget())
+getTarget().copyAuxTarget(Aux);
+
   // rewriter project will change target built-in bool type from its default.
   if (getFrontendOpts().ProgramAction == frontend::RewriteObjC)
 getTarget().noSignedCharForObjCBool();
Index: lib/Basic/Targets/AMDGPU.cpp
===
--- lib/Basic/Targets/AMDGPU.cpp
+++ lib/Basic/Targets/AMDGPU.cpp
@@ -261,6 +261,7 @@
   }
 
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+  ShouldCopyAuxTarget = true;
 }
 
 void AMDGPUTargetInfo::adjust(LangOptions ) {
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -130,6 +130,7 @@
   // Default to an unknown platform name.
   PlatformName = "unknown";
   PlatformMinVersion = VersionTuple();
+  ShouldCopyAuxTarget = false;
 }
 
 // Out of line virtual dtor for TargetInfo.
@@ -796,3 +797,12 @@
   assert(getAccumIBits() >= getUnsignedAccumIBits());
   assert(getLongAccumIBits() >= getUnsignedLongAccumIBits());
 }
+
+void TargetInfo::copyAuxTarget(TargetInfo *Aux) {
+  if (!ShouldCopyAuxTarget)
+return;
+
+  auto *Target = static_cast(this);
+  auto *Src = static_cast(Aux);
+  *Target = *Src;
+}
Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -49,21 +49,8 @@
 
 namespace Builtin { struct Info; }
 
-/// Exposes information about the current target.
-///
-class TargetInfo : public RefCountedBase {
-  std::shared_ptr 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 {
   unsigned char PointerWidth, PointerAlign;
   unsigned char BoolWidth, BoolAlign;
   unsigned char IntWidth, IntAlign;
@@ -104,15 +91,91 @@
   unsigned char SuitableAlign;
   unsigned char DefaultAlignForAttributeAligned;
   unsigned char MinGlobalAlign;
-  unsigned char MaxAtomicPromoteWidth, MaxAtomicInlineWidth;
+
+  unsigned short NewAlign;
   unsigned short MaxVectorAlign;
   unsigned short MaxTLSAlign;
+
+  const llvm::fltSemantics *HalfFormat, *FloatFormat, *DoubleFormat,
+*LongDoubleFormat, *Float128Format;
+
+  ///=== Target Data Type Query Methods ---===//
+  enum IntType {
+NoInt = 0,
+SignedChar,
+UnsignedChar,
+SignedShort,
+UnsignedShort,
+SignedInt,
+UnsignedInt,
+SignedLong,
+UnsignedLong,
+SignedLongLong,
+UnsignedLongLong
+  };
+
+  enum RealType {
+NoFloat = 255,
+Float = 0,
+Double,
+LongDouble,
+Float128
+  };
+protected:
+  IntType SizeType, IntMaxType, PtrDiffType, IntPtrType, WCharType,
+  WIntType, Char16Type, Char32Type, Int64Type, SigAtomicType,
+  ProcessIDType;
+
+  /// Whether Objective-C's built-in boolean type should be signed char.
+  ///
+  /// Otherwise, when this flag is not set, the normal built-in boolean type is
+  /// used.
+  unsigned UseSignedCharForObjCBool : 1;
+
+  /// Control whether the alignment of bit-field types is respected when laying
+  /// out structures. If true, then the alignment of the bit-field type will be
+  /// used to (a) impact the alignment of the containing structure, and (b)
+  /// ensure that the 

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

It's pretty unfortunate that all these fields have to be individually called 
out like this.  Can you move all these basic layout fields into a separate 
`struct` (which can be a secondary base class of `TargetInfo`) which can then 
just be normally copied?  Anything that needs special copy semantics, like the 
LLVM `DataLayout` (do you need to copy this?) doesn't need to go into that 
struct, just the basic POD things that determine fundamental type layouts and 
semantics.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 181326.
yaxunl added a comment.
Herald added a subscriber: jfb.

Copy type information from AuxTarget.


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

https://reviews.llvm.org/D56318

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets/AMDGPU.cpp
  lib/Frontend/CompilerInstance.cpp
  test/SemaCUDA/amdgpu-size_t.cu

Index: test/SemaCUDA/amdgpu-size_t.cu
===
--- /dev/null
+++ test/SemaCUDA/amdgpu-size_t.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple x86_64-pc-windows-msvc -fms-compatibility -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+typedef unsigned __int64 size_t;
+typedef __int64 intptr_t;
+typedef unsigned __int64 uintptr_t;
+
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -929,6 +929,8 @@
   // Adjust target options based on codegen options.
   getTarget().adjustTargetOptions(getCodeGenOpts(), getTargetOpts());
 
+  getTarget().copyAuxTarget(getAuxTarget());
+
   // rewriter project will change target built-in bool type from its default.
   if (getFrontendOpts().ProgramAction == frontend::RewriteObjC)
 getTarget().noSignedCharForObjCBool();
Index: lib/Basic/Targets/AMDGPU.cpp
===
--- lib/Basic/Targets/AMDGPU.cpp
+++ lib/Basic/Targets/AMDGPU.cpp
@@ -260,6 +260,7 @@
   }
 
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+  ShouldCopyAuxTarget = true;
 }
 
 void AMDGPUTargetInfo::adjust(LangOptions ) {
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -130,6 +130,7 @@
   // Default to an unknown platform name.
   PlatformName = "unknown";
   PlatformMinVersion = VersionTuple();
+  ShouldCopyAuxTarget = false;
 }
 
 // Out of line virtual dtor for TargetInfo.
@@ -796,3 +797,81 @@
   assert(getAccumIBits() >= getUnsignedAccumIBits());
   assert(getLongAccumIBits() >= getUnsignedLongAccumIBits());
 }
+
+void TargetInfo::copyAuxTarget(TargetInfo *Aux) {
+  if (!ShouldCopyAuxTarget)
+return;
+
+  PointerWidth = Aux->PointerWidth;
+  PointerAlign = Aux->PointerAlign;
+  BoolWidth = Aux->BoolWidth;
+  BoolAlign = Aux->BoolAlign;
+  IntWidth = Aux->IntWidth;
+  IntAlign = Aux->IntAlign;
+  LongWidth = Aux->LongWidth;
+  LongAlign = Aux->LongAlign;
+  LongLongWidth = Aux->LongLongWidth;
+  LongLongAlign = Aux->LongLongAlign;
+
+  // Fixed point default bit widths
+  ShortAccumWidth = Aux->ShortAccumWidth;
+  ShortAccumAlign = Aux->ShortAccumAlign;
+  AccumWidth = Aux->AccumWidth;
+  AccumAlign = Aux->AccumAlign;
+  LongAccumWidth = Aux->LongAccumWidth;
+  LongAccumAlign = Aux->LongAccumAlign;
+  ShortFractWidth = Aux->ShortFractWidth;
+  ShortFractAlign = Aux->ShortFractAlign;
+  FractWidth = Aux->FractWidth;
+  FractAlign = Aux->FractAlign;
+  LongFractWidth = Aux->LongFractWidth;
+  LongFractAlign = Aux->LongFractAlign;
+
+  // Fixed point default integral and fractional bit sizes
+  PaddingOnUnsignedFixedPoint = Aux->PaddingOnUnsignedFixedPoint;
+  ShortAccumScale = Aux->ShortAccumScale;
+  AccumScale = Aux->AccumScale;
+  LongAccumScale = Aux->LongAccumScale;
+
+  SuitableAlign = Aux->SuitableAlign;
+  DefaultAlignForAttributeAligned = Aux->DefaultAlignForAttributeAligned;
+  MinGlobalAlign = Aux->MinGlobalAlign;
+
+  NewAlign = Aux->NewAlign;
+
+  HalfWidth = Aux->HalfWidth;
+  HalfAlign = Aux->HalfAlign;
+  FloatWidth = Aux->FloatWidth;
+  FloatAlign = Aux->FloatAlign;
+  DoubleWidth = Aux->DoubleWidth;
+  DoubleAlign = Aux->DoubleAlign;
+  LongDoubleWidth = Aux->LongDoubleWidth;
+  LongDoubleAlign = Aux->LongDoubleAlign;
+  Float128Align = Aux->Float128Align;
+  LargeArrayMinWidth = Aux->LargeArrayMinWidth;
+  LargeArrayAlign = Aux->LargeArrayAlign;
+  MaxVectorAlign = Aux->MaxVectorAlign;
+  MaxTLSAlign = Aux->MaxTLSAlign;
+
+  SizeType = Aux->SizeType;
+  PtrDiffType = Aux->PtrDiffType;
+  IntMaxType = Aux->IntMaxType;
+  IntPtrType = Aux->IntPtrType;
+  WCharType = Aux->WCharType;
+  WIntType = Aux->WIntType;
+  Char16Type = Aux->Char16Type;
+  Char32Type = Aux->Char32Type;
+  Int64Type = Aux->Int64Type;
+  SigAtomicType = Aux->SigAtomicType;
+  ProcessIDType = Aux->ProcessIDType;
+  UseSignedCharForObjCBool = Aux->UseSignedCharForObjCBool;
+  UseBitFieldTypeAlignment = Aux->UseBitFieldTypeAlignment;
+  UseZeroLengthBitfieldAlignment = Aux->UseZeroLengthBitfieldAlignment;
+  UseExplicitBitFieldAlignment = Aux->UseExplicitBitFieldAlignment;
+  ZeroLengthBitfieldBoundary = Aux->ZeroLengthBitfieldBoundary;
+  HalfFormat = Aux->HalfFormat;
+  FloatFormat = Aux->FloatFormat;
+  DoubleFormat = Aux->DoubleFormat;
+  LongDoubleFormat = Aux->LongDoubleFormat;
+  Float128Format 

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D56318#1353176 , @rjmccall wrote:

> In D56318#1353116 , @yaxunl wrote:
>
> > In 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D56318#1353116 , @yaxunl wrote:

> In 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.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In 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.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

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`.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D56318#1352962 , @rjmccall wrote:

> If I was only concerned about `size_t`, your current solution would be fine.  
> My concern is that you really need to match *all* of the associated CPU 
> target's ABI choices, so your target really ought to be forwarding everything 
> to that target by default and only selectively overriding it in order to 
> support GPU-specific features.   Probably the easiest way to do that is via 
> inheritance.


We only need to match the type size and alignment in device and host 
compilation, but do not need to match function call ABI. In fact our backend 
has its own function ABI which is different from host on linux, but it does not 
preventing us from supporting HIP on linux. This is because the device kernel 
is launched through HIP runtime, which gets kernel argument size and offset 
from kernel image, and lays out the arguments for the kernel.

The latest CUDA kernel launching API cuLaunchKernel 
(https://docs.nvidia.com/cuda/cuda-driver-api/group__CUDA__EXEC.html#group__CUDA__EXEC_1gb8f3dc3031b40da29d5f9a7139e52e15)
 . Basically the host code only needs to pass an array of pointer to the 
arguments, whereas "the number of kernel parameters and their offsets and sizes 
do not need to be specified as that information is retrieved directly from the 
kernel's image".

If the device backend has to switch to different ABI according to host 
environment, that will be very painful for the backend.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

If I was only concerned about `size_t`, your current solution would be fine.  
My concern is that you really need to match *all* of the associated CPU 
target's ABI choices, so your target really ought to be forwarding everything 
to that target by default and only selectively overriding it in order to 
support GPU-specific features.   Probably the easiest way to do that is via 
inheritance.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D56318#1346991 , @rjmccall wrote:

> Okay.  Is there a reasonable way to make your targets delegate to a different 
> `TargetInfo` implementation for most things so that you can generally match 
> the host target for things like type sizes and alignments?


There is TargetInfo for AuxTarget. In this case, the main target is amdgpu and 
the AuxTarget is x86_64. I am thinking maybe I can add a SizeTTarget pointer to 
ASTContext, and add a hook shouldDelegateSizeTTypeToAuxTarget to TargetInfo. If 
it is true, then ASTContext use size_t type in AuxTarget.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay.  Is there a reasonable way to make your targets delegate to a different 
`TargetInfo` implementation for most things so that you can generally match the 
host target for things like type sizes and alignments?


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D56318#1346693 , @rjmccall wrote:

> No, no, I understand that you're not changing pointer sizes, but this is one 
> example of trying to match the ABI of the target environment, and I'm trying 
> to understand how far that goes.  What does it mean to be in the "MSVC" 
> environment when you're actually just compiling for the GPU?  Why are you 
> using OS headers in the first place?  Do you need struct layout to match MSVC 
> (presumably on x86-64)?  What should happen with the C++ ABI?


HIP is single source program. The same source code is compiled for both host 
and device. Since HIP is an extension to C++, it uses the C++ header files of 
the system. This is true for both host code and device code. On linux, both 
uses gcc header files. On windows, when MSVC is installed and default target 
environment is MSVC, the host compilation will use MSVC header files, so does 
the device compilation. For device compilation, most of the stuff in MSVC 
headers do not matter, e.g. function declarations, since they are for host. 
What matters are mostly type definitions. They should be consistent for both 
device and host. Since MSVC supports C++11, it should work. As an example, CUDA 
SDK supports MSVC.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

No, no, I understand that you're not changing pointer sizes, but this is one 
example of trying to match the ABI of the target environment, and I'm trying to 
understand how far that goes.  What does it mean to be in the "MSVC" 
environment when you're actually just compiling for the GPU?  Why are you using 
OS headers in the first place?  Do you need struct layout to match MSVC 
(presumably on x86-64)?  What should happen with the C++ ABI?


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D56318#1346456 , @rjmccall wrote:

> What's the general idea here, that you're going to pretend to be the 
> environment's "standard" CPU target of the right pointer width and try to 
> match the ABI exactly?  This seems like a pretty treacherous road to go down.


The pointer width does not change. In both case it is 64 bit. The only 
difference is that MSVC uses unsigned long long as size_t whereas by default 
AMDGPU uses unsigned long as size_t. They have the same size but in AST they 
are different type. When HIP is compiled in MSVC environment, it has to use 
header files of MSVC. This nominal difference in size_t definition causes 
compilation error since MSVC header files contains typedef of size_t as 
unsigned long long. Since we cannot change header files of MSVC, we have to 
change our own size_t definition.

We do not want to change our device ABI.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

What's the general idea here, that you're going to pretend to be the 
environment's "standard" CPU target of the right pointer width and try to match 
the ABI exactly?  This seems like a pretty treacherous road to go down.


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

https://reviews.llvm.org/D56318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall.
Herald added subscribers: tpr, nhaehnle, jvesely.

In 64 bit MSVC environment size_t is defined as unsigned long long. Fix AMDGPU 
target info to match it in MSVC environment.


https://reviews.llvm.org/D56318

Files:
  lib/Basic/Targets/AMDGPU.cpp
  lib/Driver/Driver.cpp
  test/SemaCUDA/amdgpu-size_t.cu


Index: test/SemaCUDA/amdgpu-size_t.cu
===
--- /dev/null
+++ test/SemaCUDA/amdgpu-size_t.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa-msvc -fms-compatibility 
-fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+typedef unsigned __int64 size_t;
+typedef __int64 intptr_t;
+typedef unsigned __int64 uintptr_t;
+
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -614,7 +614,11 @@
 StringRef DeviceTripleStr;
 auto OFK = Action::OFK_HIP;
 DeviceTripleStr = "amdgcn-amd-amdhsa";
-llvm::Triple HIPTriple(DeviceTripleStr);
+StringRef Env;
+if (HostTriple.getEnvironment() == llvm::Triple::MSVC)
+  Env = HostTriple.getEnvironmentName();
+llvm::Triple HIPTriple(Env.empty() ? Twine(DeviceTripleStr) :
+DeviceTripleStr + "-" + Env);
 // Use the HIP and host triples as the key into the ToolChains map,
 // because the device toolchain we create depends on both.
 auto  = ToolChains[HIPTriple.str() + "/" + HostTriple.str()];
Index: lib/Basic/Targets/AMDGPU.cpp
===
--- lib/Basic/Targets/AMDGPU.cpp
+++ lib/Basic/Targets/AMDGPU.cpp
@@ -254,9 +254,15 @@
   PointerWidth = PointerAlign = DataLayout->getPointerSizeInBits();
   if (getMaxPointerWidth() == 64) {
 LongWidth = LongAlign = 64;
-SizeType = UnsignedLong;
-PtrDiffType = SignedLong;
-IntPtrType = SignedLong;
+if (Triple.getEnvironment() == llvm::Triple::MSVC) {
+  SizeType = UnsignedLongLong;
+  PtrDiffType = SignedLongLong;
+  IntPtrType = SignedLongLong;
+} else {
+  SizeType = UnsignedLong;
+  PtrDiffType = SignedLong;
+  IntPtrType = SignedLong;
+}
   }
 
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;


Index: test/SemaCUDA/amdgpu-size_t.cu
===
--- /dev/null
+++ test/SemaCUDA/amdgpu-size_t.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa-msvc -fms-compatibility -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+typedef unsigned __int64 size_t;
+typedef __int64 intptr_t;
+typedef unsigned __int64 uintptr_t;
+
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -614,7 +614,11 @@
 StringRef DeviceTripleStr;
 auto OFK = Action::OFK_HIP;
 DeviceTripleStr = "amdgcn-amd-amdhsa";
-llvm::Triple HIPTriple(DeviceTripleStr);
+StringRef Env;
+if (HostTriple.getEnvironment() == llvm::Triple::MSVC)
+  Env = HostTriple.getEnvironmentName();
+llvm::Triple HIPTriple(Env.empty() ? Twine(DeviceTripleStr) :
+DeviceTripleStr + "-" + Env);
 // Use the HIP and host triples as the key into the ToolChains map,
 // because the device toolchain we create depends on both.
 auto  = ToolChains[HIPTriple.str() + "/" + HostTriple.str()];
Index: lib/Basic/Targets/AMDGPU.cpp
===
--- lib/Basic/Targets/AMDGPU.cpp
+++ lib/Basic/Targets/AMDGPU.cpp
@@ -254,9 +254,15 @@
   PointerWidth = PointerAlign = DataLayout->getPointerSizeInBits();
   if (getMaxPointerWidth() == 64) {
 LongWidth = LongAlign = 64;
-SizeType = UnsignedLong;
-PtrDiffType = SignedLong;
-IntPtrType = SignedLong;
+if (Triple.getEnvironment() == llvm::Triple::MSVC) {
+  SizeType = UnsignedLongLong;
+  PtrDiffType = SignedLongLong;
+  IntPtrType = SignedLongLong;
+} else {
+  SizeType = UnsignedLong;
+  PtrDiffType = SignedLong;
+  IntPtrType = SignedLong;
+}
   }
 
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits