[clang] [CUDA][HIP] Fix record layout on Windows (PR #87651)

2024-04-17 Thread Yaxun Liu via cfe-commits

https://github.com/yxsamliu closed 
https://github.com/llvm/llvm-project/pull/87651
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [CUDA][HIP] Fix record layout on Windows (PR #87651)

2024-04-17 Thread Artem Belevich via cfe-commits

https://github.com/Artem-B approved this pull request.


https://github.com/llvm/llvm-project/pull/87651
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [CUDA][HIP] Fix record layout on Windows (PR #87651)

2024-04-17 Thread Yaxun Liu via cfe-commits

yxsamliu wrote:

ping

It passes our internal Windows CI.

https://github.com/llvm/llvm-project/pull/87651
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [CUDA][HIP] Fix record layout on Windows (PR #87651)

2024-04-10 Thread Yaxun Liu via cfe-commits

yxsamliu wrote:

> In general, having different C++ ABIs between the host and device seems like 
> an ongoing source of tension and bugs.

I agree. However completely switching to Microsoft ABI on device side does not 
work with existing device libraries since they assume Itanium mangling. 
Therefore I only changes record layout to be compatible with host, in the hope 
that the generated LLVM IR is correct for such a combination.

I added more tests about member accessing and virtual function calls. It seems 
the IR is correct. I think clang codegen is generic enough to handle Itanium 
ABI with Microsoft record layout.



https://github.com/llvm/llvm-project/pull/87651
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [CUDA][HIP] Fix record layout on Windows (PR #87651)

2024-04-10 Thread Yaxun Liu via cfe-commits

https://github.com/yxsamliu updated 
https://github.com/llvm/llvm-project/pull/87651

>From fcebb8226599160fd6e6e42356f78d0b1d4367d4 Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu" 
Date: Thu, 4 Apr 2024 12:09:04 -0400
Subject: [PATCH] [CUDA][HIP] Fix record layout on Windows

On windows, record layout should be consistent with
host side, otherwise host code is no able to access
fields of the record correctly.

Fixes: https://github.com/llvm/llvm-project/issues/51031

Fixes: SWDEV-446010
Change-Id: Id590a7d3bc0b6fd0ea745cf2a049e1f89ae134fa
---
 clang/lib/AST/RecordLayoutBuilder.cpp   |   5 +
 clang/test/CodeGenCUDA/record-layout.cu | 200 
 2 files changed, 205 insertions(+)
 create mode 100644 clang/test/CodeGenCUDA/record-layout.cu

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index a3b7431f7ffd6d..d9bf62c2bbb04a 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2458,6 +2458,11 @@ static bool mustSkipTailPadding(TargetCXXABI ABI, const 
CXXRecordDecl *RD) {
 }
 
 static bool isMsLayout(const ASTContext ) {
+  // Check if it's CUDA device compilation; ensure layout consistency with 
host.
+  if (Context.getLangOpts().CUDA && Context.getLangOpts().CUDAIsDevice &&
+  Context.getAuxTargetInfo())
+return Context.getAuxTargetInfo()->getCXXABI().isMicrosoft();
+
   return Context.getTargetInfo().getCXXABI().isMicrosoft();
 }
 
diff --git a/clang/test/CodeGenCUDA/record-layout.cu 
b/clang/test/CodeGenCUDA/record-layout.cu
new file mode 100644
index 00..dd34121ccb9d36
--- /dev/null
+++ b/clang/test/CodeGenCUDA/record-layout.cu
@@ -0,0 +1,200 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fdump-record-layouts \
+// RUN:   -emit-llvm -o %t -xhip %s 2>&1 | FileCheck %s --check-prefix=AST
+// RUN: cat %t | FileCheck --check-prefixes=CHECK,HOST %s
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -target-cpu 
gfx1100 \
+// RUN:   -emit-llvm -fdump-record-layouts -aux-triple x86_64-pc-windows-msvc \
+// RUN:   -o %t -xhip %s | FileCheck %s --check-prefix=AST
+// RUN: cat %t | FileCheck --check-prefixes=CHECK,DEV %s
+
+#include "Inputs/cuda.h"
+
+// AST: *** Dumping AST Record Layout
+// AST-LABEL: 0 | struct C
+// AST-NEXT:  0 |   struct A (base) (empty)
+// AST-NEXT:  1 |   struct B (base) (empty)
+// AST-NEXT:  4 |   int i
+// AST-NEXT:| [sizeof=8, align=4,
+// AST-NEXT:|  nvsize=8, nvalign=4]
+
+// CHECK: %struct.C = type { [4 x i8], i32 }
+
+struct A {};
+struct B {};
+struct C : A, B {
+int i;
+};
+
+// AST: *** Dumping AST Record Layout
+// AST-LABEL:  0 | struct I
+// AST-NEXT:   0 |   (I vftable pointer)
+// AST-NEXT:   8 |   int i
+// AST-NEXT: | [sizeof=16, align=8,
+// AST-NEXT: |  nvsize=16, nvalign=8]
+
+// AST: *** Dumping AST Record Layout
+// AST-LABEL:  0 | struct J
+// AST-NEXT:   0 |   struct I (primary base)
+// AST-NEXT:   0 | (I vftable pointer)
+// AST-NEXT:   8 | int i
+// AST-NEXT:  16 |   int j
+// AST-NEXT: | [sizeof=24, align=8,
+// AST-NEXT: |  nvsize=24, nvalign=8]
+
+// CHECK: %struct.I = type { ptr, i32 }
+// CHECK: %struct.J = type { %struct.I, i32 }
+
+// HOST: @0 = private unnamed_addr constant { [4 x ptr] } { [4 x ptr] [ptr 
@"??_R4J@@6B@", ptr @"?f@J@@UEAAXXZ", ptr null, ptr @"?h@J@@UEAAXXZ"] }, 
comdat($"??_7J@@6B@")
+// HOST: @1 = private unnamed_addr constant { [4 x ptr] } { [4 x ptr] [ptr 
@"??_R4I@@6B@", ptr @_purecall, ptr null, ptr @_purecall] }, 
comdat($"??_7I@@6B@")
+// HOST: @"??_7J@@6B@" = unnamed_addr alias ptr, getelementptr inbounds ({ [4 
x ptr] }, ptr @0, i32 0, i32 0, i32 1)
+// HOST: @"??_7I@@6B@" = unnamed_addr alias ptr, getelementptr inbounds ({ [4 
x ptr] }, ptr @1, i32 0, i32 0, i32 1)
+
+// DEV: @_ZTV1J = linkonce_odr unnamed_addr addrspace(1) constant { [5 x ptr 
addrspace(1)] } { [5 x ptr addrspace(1)] [ptr addrspace(1) null, ptr 
addrspace(1) null, ptr addrspace(1) null, ptr addrspace(1) addrspacecast (ptr 
@_ZN1J1gEv to ptr addrspace(1)), ptr addrspace(1) addrspacecast (ptr @_ZN1J1hEv 
to ptr addrspace(1))] }, comdat, align 8
+// DEV: @_ZTV1I = linkonce_odr unnamed_addr addrspace(1) constant { [5 x ptr 
addrspace(1)] } { [5 x ptr addrspace(1)] [ptr addrspace(1) null, ptr 
addrspace(1) null, ptr addrspace(1) null, ptr addrspace(1) addrspacecast (ptr 
@__cxa_pure_virtual to ptr addrspace(1)), ptr addrspace(1) addrspacecast (ptr 
@__cxa_pure_virtual to ptr addrspace(1))] }, comdat, align 8
+struct I {
+virtual void f() = 0;
+__device__ virtual void g() = 0;
+__device__ __host__ virtual void h() = 0;
+int i;
+};
+
+struct J : I {
+void f() override {}
+__device__ void g() override {}
+__device__ __host__ void h() override {}
+int j;
+};
+
+// DEV: define dso_local amdgpu_kernel void 

[clang] [CUDA][HIP] Fix record layout on Windows (PR #87651)

2024-04-05 Thread Yaxun Liu via cfe-commits

yxsamliu wrote:

> > Keeping layout in sync makes sense to me, but I'm completely unfamiliar 
> > with the windows side.
> > @rnk is there anything else we need to worry about?
> 
> I checked, and I think this routes everything over to the MS record layout 
> builder, so it should be comprehensive:
> 
> https://github.com/llvm/llvm-project/blob/d97d560fbf6ed26a198b3afe1594d7d63b88ab3a/clang/lib/AST/RecordLayoutBuilder.cpp#L3354
> 
> I would augment the test a bit, but otherwise this looks good to me.

will add more tests about field access and virtual function calls

https://github.com/llvm/llvm-project/pull/87651
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits