[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-08-07 Thread Alex Bradbury via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe3c57fdd8439: [clang][RISCV] Fix bug in ABI handling of 
empty structs with hard FP calling… (authored by asb).

Changed prior to commit:
  https://reviews.llvm.org/D142327?vs=546012=547684#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142327

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/ABIInfoImpl.cpp
  clang/lib/CodeGen/ABIInfoImpl.h
  clang/lib/CodeGen/Targets/RISCV.cpp
  clang/test/CodeGen/RISCV/abi-empty-structs.c

Index: clang/test/CodeGen/RISCV/abi-empty-structs.c
===
--- clang/test/CodeGen/RISCV/abi-empty-structs.c
+++ clang/test/CodeGen/RISCV/abi-empty-structs.c
@@ -1,4 +1,4 @@
-// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --full-function-signature --filter "^define |^entry:"
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --filter "^define |^entry:" --version 2
 // RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-abi ilp32f -emit-llvm %s -o - \
 // RUN:   | FileCheck -check-prefixes=CHECK-C,CHECK32-C %s
 // RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-feature +d -target-abi ilp32d -emit-llvm %s -o - \
@@ -19,8 +19,9 @@
 #include 
 
 // Fields containing empty structs or unions are ignored when flattening
-// structs for the hard FP ABIs, even in C++.
-// FIXME: This isn't currently respected.
+// structs for the hard FP ABIs, even in C++. The rules for arrays of empty
+// structs or unions are subtle and documented in
+// .
 
 struct empty { struct { struct { } e; }; };
 struct s1 { struct empty e; float f; };
@@ -29,13 +30,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local [2 x i32] @_Z7test_s12s1
-// CHECK32-CXX-SAME: ([2 x i32] [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s12s1
-// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local float @_Z7test_s12s1
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-CXX:  entry:
 //
 struct s1 test_s1(struct s1 a) {
   return a;
@@ -47,13 +44,9 @@
 // CHECK-C-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s22s2
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S2:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s22s2
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { i32, float } @_Z7test_s22s2
+// CHECK-CXX-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s2 test_s2(struct s2 a) {
   return a;
@@ -65,13 +58,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s32s3
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S3:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s32s3
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s32s3
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s3 test_s3(struct s3 a) {
   return a;
@@ -83,13 +72,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s42s4
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S4:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s42s4
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s42s4
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s4 test_s4(struct s4 a) {
   return a;
@@ -142,7 +127,7 @@
 // CHECK-C:  entry:
 //
 // CHECK-CXX-LABEL: define dso_local float @_Z7test_s72s7
-// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
 // CHECK-CXX:  entry:
 //
 struct s7 test_s7(struct s7 a) {
@@ -156,17 +141,31 @@
 // CHECK-C-SAME: (float

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-08-07 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits 
rogfer01 accepted this revision.
rogfer01 added a comment.
This revision is now accepted and ready to land.

Thanks for the update @asb. LGTM.


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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-08-01 Thread Alex Bradbury via Phabricator via cfe-commits 
asb updated this revision to Diff 546012.
asb added a comment.

This update includes test coverage for the bug that was fixed in the reverted 
version of the patch.

Thanks @rogfer01 for the smaller test case. To my surprise, cvise stripped out 
almost everything and test_s9 in abi-empty-structs.c reliably caused the assert 
for me.

Re-review appreciated in order to get this landed and ideally cherry-picked for 
the 17.x.


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

https://reviews.llvm.org/D142327

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/ABIInfoImpl.cpp
  clang/lib/CodeGen/ABIInfoImpl.h
  clang/lib/CodeGen/Targets/RISCV.cpp
  clang/test/CodeGen/RISCV/abi-empty-structs.c

Index: clang/test/CodeGen/RISCV/abi-empty-structs.c
===
--- clang/test/CodeGen/RISCV/abi-empty-structs.c
+++ clang/test/CodeGen/RISCV/abi-empty-structs.c
@@ -1,4 +1,4 @@
-// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --full-function-signature --filter "^define |^entry:"
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --filter "^define |^entry:" --version 2
 // RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-abi ilp32f -emit-llvm %s -o - \
 // RUN:   | FileCheck -check-prefixes=CHECK-C,CHECK32-C %s
 // RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-feature +d -target-abi ilp32d -emit-llvm %s -o - \
@@ -19,8 +19,9 @@
 #include 
 
 // Fields containing empty structs or unions are ignored when flattening
-// structs for the hard FP ABIs, even in C++.
-// FIXME: This isn't currently respected.
+// structs for the hard FP ABIs, even in C++. The rules for arrays of empty
+// structs or unions are subtle and documented in
+// .
 
 struct empty { struct { struct { } e; }; };
 struct s1 { struct empty e; float f; };
@@ -29,13 +30,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local [2 x i32] @_Z7test_s12s1
-// CHECK32-CXX-SAME: ([2 x i32] [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s12s1
-// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local float @_Z7test_s12s1
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-CXX:  entry:
 //
 struct s1 test_s1(struct s1 a) {
   return a;
@@ -47,13 +44,9 @@
 // CHECK-C-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s22s2
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S2:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s22s2
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { i32, float } @_Z7test_s22s2
+// CHECK-CXX-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s2 test_s2(struct s2 a) {
   return a;
@@ -65,13 +58,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s32s3
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S3:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s32s3
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s32s3
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s3 test_s3(struct s3 a) {
   return a;
@@ -83,13 +72,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s42s4
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S4:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s42s4
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s42s4
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s4 test_s4(struct s4 a) {
   return a;
@@ -142,7 +127,7 @@
 // CHECK-C:  entry:
 //
 // CHECK-CXX-LABEL: define dso_local float @_Z7test_s72s7
-// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
 // CHECK-CXX:

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-07-27 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits 
rogfer01 added a comment.

I didn't have libcxx handy but @SixWeining testcase also fails with libstdcxx 
so I expanded what C++ does and removed the templates.

With the new patch this does not fail anymore.

  typedef decltype((int *)2 - (int *)1) my_ptrdiff_t;
  
  struct my_lambda {
bool operator()(int i) { return i == 2; }
  };
  
  struct my_iter_pred {
my_lambda _M_pred;
  
explicit my_iter_pred(my_lambda __pred)
: _M_pred(/* move */ static_cast(__pred)) {}
  
bool operator()(int *__it) { return bool(_M_pred(*__it)); }
  };
  
  my_ptrdiff_t __my_count_if(int *__first, int *__last, my_iter_pred __pred) {
my_ptrdiff_t __n = 0;
for (; __first != __last; ++__first)
  if (__pred(__first))
++__n;
return __n;
  }
  
  inline my_iter_pred my_pred_iter(my_lambda __my_lambda) {
return my_iter_pred(/* move */ static_cast(__my_lambda));
  }
  
  inline my_ptrdiff_t my_count_if(int *__first, int *__last,
  my_lambda __my_lambda) {
return __my_count_if(__first, __last, my_pred_iter(__my_lambda));
  }
  
  int main() {
int v[] = {1, 2, 3, 2, 2};
return my_count_if(v, v + sizeof(v) / sizeof(*v), my_lambda());
  }

I tried to remove the `my_pred_iter` / `my_iter_pred` wrapper but then the 
original patch would not fail anymore.

Hope this helps.


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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-07-27 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits 
rogfer01 added a comment.

I didn't have libcxx handy but @SixWeining testcase also failed with libstdcxx 
so I made a slightly smaller standalone testcase based on what libstdcxx does 
and I manually replaced all the template stuff.

  typedef decltype((int *)2 - (int *)1) my_ptrdiff_t;
  
  struct my_lambda {
bool operator()(int i) { return i == 2; }
  };
  
  struct my_iter_pred {
my_lambda _M_pred;
  
bool operator()(int *__it) { return bool(_M_pred(*__it)); }
  };
  
  my_ptrdiff_t __my_count_if(int *__first, int *__last, my_iter_pred __pred) {
my_ptrdiff_t __n = 0;
for (; __first != __last; ++__first)
  if (__pred(__first))
++__n;
return __n;
  }
  
  inline my_iter_pred my_pred_iter(my_lambda __my_lambda) {
return my_iter_pred(/* move */ static_cast(_my_lambda));
  }
  
  inline my_ptrdiff_t my_count_if(int *__first, int *__last,
  my_lambda __my_lambda) {
return __my_count_if(__first, __last, my_pred_iter(__my_lambda));
  }
  
  int main() {
int v[] = {1, 2, 3, 2, 2};
return my_count_if(v, v + sizeof(v) / sizeof(*v), my_lambda());
  }

I tried to simplify it further by removing the `my_pred_iter` / `my_iter_pred` 
adapter but then it would not crash anymore. Hope this helps.


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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-07-26 Thread Alex Bradbury via Phabricator via cfe-commits 
asb updated this revision to Diff 544349.
asb added a comment.

I've updated the patch to fix the reported issue (adding an additional check to 
detectFPCCEligibleStruct). What I haven't been able to do so far is to extract 
a test case that shows the issue indicated by @SixWeining without relying on 
libcxx headers. I may well be missing something obvious here.


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

https://reviews.llvm.org/D142327

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/ABIInfoImpl.cpp
  clang/lib/CodeGen/ABIInfoImpl.h
  clang/lib/CodeGen/Targets/RISCV.cpp
  clang/test/CodeGen/RISCV/abi-empty-structs.c

Index: clang/test/CodeGen/RISCV/abi-empty-structs.c
===
--- clang/test/CodeGen/RISCV/abi-empty-structs.c
+++ clang/test/CodeGen/RISCV/abi-empty-structs.c
@@ -19,8 +19,9 @@
 #include 
 
 // Fields containing empty structs or unions are ignored when flattening
-// structs for the hard FP ABIs, even in C++.
-// FIXME: This isn't currently respected.
+// structs for the hard FP ABIs, even in C++. The rules for arrays of empty
+// structs or unions are subtle and documented in
+// .
 
 struct empty { struct { struct { } e; }; };
 struct s1 { struct empty e; float f; };
@@ -29,13 +30,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local [2 x i32] @_Z7test_s12s1
-// CHECK32-CXX-SAME: ([2 x i32] [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s12s1
-// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local float @_Z7test_s12s1
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-CXX:  entry:
 //
 struct s1 test_s1(struct s1 a) {
   return a;
@@ -47,13 +44,9 @@
 // CHECK-C-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s22s2
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S2:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s22s2
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { i32, float } @_Z7test_s22s2
+// CHECK-CXX-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s2 test_s2(struct s2 a) {
   return a;
@@ -65,13 +58,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s32s3
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S3:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s32s3
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s32s3
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s3 test_s3(struct s3 a) {
   return a;
@@ -83,13 +72,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s42s4
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S4:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s42s4
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s42s4
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s4 test_s4(struct s4 a) {
   return a;
@@ -142,7 +127,7 @@
 // CHECK-C:  entry:
 //
 // CHECK-CXX-LABEL: define dso_local float @_Z7test_s72s7
-// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
 // CHECK-CXX:  entry:
 //
 struct s7 test_s7(struct s7 a) {
@@ -156,13 +141,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local i32 @_Z7test_s82s8
-// CHECK32-CXX-SAME: (i32 [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s82s8
-// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local float @_Z7test_s82s8
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s8 test_s8(struct s8 a) {
   return a;
Index:

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-07-24 Thread Lu Weining via Phabricator via cfe-commits 
SixWeining added inline comments.



Comment at: clang/lib/CodeGen/Targets/RISCV.cpp:178
   return false;
-if (isEmptyRecord(getContext(), Ty, true))
+if (isEmptyRecord(getContext(), Ty, true, true))
   return true;

asb wrote:
> rogfer01 wrote:
> > I've observed (based on manually added tracing) that we may return true 
> > here with `Field1Ty == nullptr` and `Field2Ty == nullptr`.
> > 
> > Then `RISCVABIInfo::coerceAndExpandFPCCEligibleStruct` receives these two 
> > types and appends `Field1Ty` it into `UnpaddedCoerceElts` and then if 
> > `Field2Ty == nullptr` it calls `ABIArgInfo::getCoerceAndExpand` (around 
> > line 280 of `clang/lib/CodeGen/Targets/RISCV.cpp`) which then tries to do a 
> > `dyn_cast` on a null value and then an assertion fires (around line 256 of 
> > `clang/include/clang/CodeGen/CGFunctionInfo.h`)
> > 
> > I can reproduce this with the llvm-testsuite. Apologies that I have not 
> > managed to create a small reproducer yet.
> Thank you, I've reverted until this can be investigated and fixed.
LoongArch has the same issue and I write a reproducer:
```
$ cat test.cpp
#include 
#include 

static bool check(int i) {
  return i == 2;
}

int main()
{
  std::vector v{1, 2, 3, 2, 2};
  //return std::count_if(v.begin(), v.end(), check); // ok
  return std::count_if(v.begin(), v.end(), [](int i) { return i == 2; }); // 
error
}
```

Let's investigate how to fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-07-24 Thread Alex Bradbury via Phabricator via cfe-commits 
asb reopened this revision.
asb added a comment.
This revision is now accepted and ready to land.

Reopening as I've reverted following Roger's bug report.




Comment at: clang/lib/CodeGen/Targets/RISCV.cpp:178
   return false;
-if (isEmptyRecord(getContext(), Ty, true))
+if (isEmptyRecord(getContext(), Ty, true, true))
   return true;

rogfer01 wrote:
> I've observed (based on manually added tracing) that we may return true here 
> with `Field1Ty == nullptr` and `Field2Ty == nullptr`.
> 
> Then `RISCVABIInfo::coerceAndExpandFPCCEligibleStruct` receives these two 
> types and appends `Field1Ty` it into `UnpaddedCoerceElts` and then if 
> `Field2Ty == nullptr` it calls `ABIArgInfo::getCoerceAndExpand` (around line 
> 280 of `clang/lib/CodeGen/Targets/RISCV.cpp`) which then tries to do a 
> `dyn_cast` on a null value and then an assertion fires (around line 256 of 
> `clang/include/clang/CodeGen/CGFunctionInfo.h`)
> 
> I can reproduce this with the llvm-testsuite. Apologies that I have not 
> managed to create a small reproducer yet.
Thank you, I've reverted until this can be investigated and fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-07-24 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits 
rogfer01 added inline comments.



Comment at: clang/lib/CodeGen/Targets/RISCV.cpp:178
   return false;
-if (isEmptyRecord(getContext(), Ty, true))
+if (isEmptyRecord(getContext(), Ty, true, true))
   return true;

I've observed (based on manually added tracing) that we may return true here 
with `Field1Ty == nullptr` and `Field2Ty == nullptr`.

Then `RISCVABIInfo::coerceAndExpandFPCCEligibleStruct` receives these two types 
and appends `Field1Ty` it into `UnpaddedCoerceElts` and then if `Field2Ty == 
nullptr` it calls `ABIArgInfo::getCoerceAndExpand` (around line 280 of 
`clang/lib/CodeGen/Targets/RISCV.cpp`) which then tries to do a `dyn_cast` on a 
null value and then an assertion fires (around line 256 of 
`clang/include/clang/CodeGen/CGFunctionInfo.h`)

I can reproduce this with the llvm-testsuite. Apologies that I have not managed 
to create a small reproducer yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-07-24 Thread Alex Bradbury via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG17a58b3ca7ec: [clang][RISCV] Fix ABI handling of empty 
structs with hard FP calling… (authored by asb).
Herald added a subscriber: wangpc.

Changed prior to commit:
  https://reviews.llvm.org/D142327?vs=491781=543432#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142327

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/ABIInfoImpl.cpp
  clang/lib/CodeGen/ABIInfoImpl.h
  clang/lib/CodeGen/Targets/RISCV.cpp
  clang/test/CodeGen/RISCV/abi-empty-structs.c

Index: clang/test/CodeGen/RISCV/abi-empty-structs.c
===
--- clang/test/CodeGen/RISCV/abi-empty-structs.c
+++ clang/test/CodeGen/RISCV/abi-empty-structs.c
@@ -19,8 +19,9 @@
 #include 
 
 // Fields containing empty structs or unions are ignored when flattening
-// structs for the hard FP ABIs, even in C++.
-// FIXME: This isn't currently respected.
+// structs for the hard FP ABIs, even in C++. The rules for arrays of empty
+// structs or unions are subtle and documented in
+// .
 
 struct empty { struct { struct { } e; }; };
 struct s1 { struct empty e; float f; };
@@ -29,13 +30,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local [2 x i32] @_Z7test_s12s1
-// CHECK32-CXX-SAME: ([2 x i32] [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s12s1
-// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local float @_Z7test_s12s1
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-CXX:  entry:
 //
 struct s1 test_s1(struct s1 a) {
   return a;
@@ -47,13 +44,9 @@
 // CHECK-C-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s22s2
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S2:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s22s2
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { i32, float } @_Z7test_s22s2
+// CHECK-CXX-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s2 test_s2(struct s2 a) {
   return a;
@@ -65,13 +58,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s32s3
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S3:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s32s3
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s32s3
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s3 test_s3(struct s3 a) {
   return a;
@@ -83,13 +72,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s42s4
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S4:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s42s4
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s42s4
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s4 test_s4(struct s4 a) {
   return a;
@@ -142,7 +127,7 @@
 // CHECK-C:  entry:
 //
 // CHECK-CXX-LABEL: define dso_local float @_Z7test_s72s7
-// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
 // CHECK-CXX:  entry:
 //
 struct s7 test_s7(struct s7 a) {
@@ -156,13 +141,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local i32 @_Z7test_s82s8
-// CHECK32-CXX-SAME: (i32 [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s82s8
-// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local float @_Z7test_s82s8
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s8

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-03-14 Thread Luís Marques via Phabricator via cfe-commits 
luismarques added inline comments.



Comment at: clang/test/CodeGen/RISCV/abi-empty-structs.c:21-22
 
 // Fields containing empty structs or unions are ignored when flattening
 // structs for the hard FP ABIs, even in C++.
-// FIXME: This isn't currently respected.

Shouldn't you mention the array exception here? (which you now cover in 
`test_s7` and `test_s8`).


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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-02-10 Thread Kito Cheng via Phabricator via cfe-commits 
kito-cheng accepted this revision.
kito-cheng added a comment.

LGTM, Verified with GCC and two clang w/ and w/o this patch, clang with this 
patch is matching GCC's behavior.

Also created an PR on psABI to clarify that: 
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/365


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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-02-07 Thread Alex Bradbury via Phabricator via cfe-commits 
asb added a comment.
Herald added a subscriber: jobnoorman.

Friendly ping on this (I think mostly directed at @kito-cheng who was hoping to 
find time to review the linked abi issue 
).


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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-01-31 Thread Alex Bradbury via Phabricator via cfe-commits 
asb added a comment.

In D142327#4080044 , @asb wrote:

> Thanks Luis for the quick review. As an important part of this is trying to 
> align with gcc/g++ I'd really appreciate a review from @kito-cheng too if you 
> have the time (thanks in advance!).

Friendly ping to Kito as I spotted you mentioned you're back from vacation!


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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-01-25 Thread Alex Bradbury via Phabricator via cfe-commits 
asb added a comment.

Thanks Luis for the quick review. As an important part of this is trying to 
align with gcc/g++ I'd really appreciate a review from @kito-cheng too if you 
have the time (thanks in advance!).


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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-01-24 Thread Alex Bradbury via Phabricator via cfe-commits 
asb updated this revision to Diff 491781.
asb marked an inline comment as done.
asb added a comment.

- Address review comments
- Refresh after adding zero-length array tests (and fix a bug this unearthed).
- Add to release notes


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

https://reviews.llvm.org/D142327

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/RISCV/abi-empty-structs.c

Index: clang/test/CodeGen/RISCV/abi-empty-structs.c
===
--- clang/test/CodeGen/RISCV/abi-empty-structs.c
+++ clang/test/CodeGen/RISCV/abi-empty-structs.c
@@ -20,7 +20,6 @@
 
 // Fields containing empty structs or unions are ignored when flattening
 // structs for the hard FP ABIs, even in C++.
-// FIXME: This isn't currently respected.
 
 struct empty { struct { struct { } e; }; };
 struct s1 { struct empty e; float f; };
@@ -29,13 +28,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local [2 x i32] @_Z7test_s12s1
-// CHECK32-CXX-SAME: ([2 x i32] [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s12s1
-// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local float @_Z7test_s12s1
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-CXX:  entry:
 //
 struct s1 test_s1(struct s1 a) {
   return a;
@@ -47,13 +42,9 @@
 // CHECK-C-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s22s2
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S2:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s22s2
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { i32, float } @_Z7test_s22s2
+// CHECK-CXX-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s2 test_s2(struct s2 a) {
   return a;
@@ -65,13 +56,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s32s3
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S3:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s32s3
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s32s3
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s3 test_s3(struct s3 a) {
   return a;
@@ -83,13 +70,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s42s4
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S4:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s42s4
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s42s4
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s4 test_s4(struct s4 a) {
   return a;
@@ -142,7 +125,7 @@
 // CHECK-C:  entry:
 //
 // CHECK-CXX-LABEL: define dso_local float @_Z7test_s72s7
-// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
 // CHECK-CXX:  entry:
 //
 struct s7 test_s7(struct s7 a) {
@@ -156,13 +139,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local i32 @_Z7test_s82s8
-// CHECK32-CXX-SAME: (i32 [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s82s8
-// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local float @_Z7test_s82s8
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s8 test_s8(struct s8 a) {
   return a;
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -548,12 +548,13 @@
   return Ctx.getOrInsertSyncScopeID(""); /* default sync scope */
 }
 
-static bool isEmptyRecord(ASTContext , QualType T, bool AllowArrays);
+static bool isEmptyRecord(ASTContext , QualType T, bool AllowArrays,
+  bool AsIfNoUniqueAddr = false);
 
 /// isEmptyField -

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-01-23 Thread Luís Marques via Phabricator via cfe-commits 
luismarques accepted this revision.
luismarques added a comment.
This revision is now accepted and ready to land.

LGMT.




Comment at: clang/lib/CodeGen/TargetInfo.cpp:591
   if (isa(RT->getDecl()) &&
-  (WasArray || !FD->hasAttr()))
+  (WasArray || (!FD->hasAttr() && !AsIfNoUniqueAddr)))
 return false;

Nit: the `!AsIfNoUniqueAddr` condition could be tested before the 
`!FD->hasAttr()` condition, and I imagine that would be 
slightly cheaper for the `AsIfNoUniqueAddr = true` case?


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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-01-22 Thread Alex Bradbury via Phabricator via cfe-commits 
asb updated this revision to Diff 491237.
asb added a comment.

Removed FIXME missed in initial version.


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

https://reviews.llvm.org/D142327

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/RISCV/abi-empty-structs.c

Index: clang/test/CodeGen/RISCV/abi-empty-structs.c
===
--- clang/test/CodeGen/RISCV/abi-empty-structs.c
+++ clang/test/CodeGen/RISCV/abi-empty-structs.c
@@ -20,7 +20,6 @@
 
 // Fields containing empty structs or unions are ignored when flattening
 // structs for the hard FP ABIs, even in C++.
-// FIXME: This isn't currently respected.
 
 struct empty { struct { struct { } e; }; };
 struct s1 { struct empty e; float f; };
@@ -29,13 +28,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local [2 x i32] @_Z7test_s12s1
-// CHECK32-CXX-SAME: ([2 x i32] [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s12s1
-// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local float @_Z7test_s12s1
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-CXX:  entry:
 //
 struct s1 test_s1(struct s1 a) {
   return a;
@@ -47,13 +42,9 @@
 // CHECK-C-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s22s2
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S2:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s22s2
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { i32, float } @_Z7test_s22s2
+// CHECK-CXX-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s2 test_s2(struct s2 a) {
   return a;
@@ -65,13 +56,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s32s3
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S3:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s32s3
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s32s3
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s3 test_s3(struct s3 a) {
   return a;
@@ -83,13 +70,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s42s4
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S4:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s42s4
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s42s4
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s4 test_s4(struct s4 a) {
   return a;
@@ -136,6 +119,5 @@
 }
 
  NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
-// CHECK-CXX: {{.*}}
 // CHECK32-C: {{.*}}
 // CHECK64-C: {{.*}}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -548,12 +548,13 @@
   return Ctx.getOrInsertSyncScopeID(""); /* default sync scope */
 }
 
-static bool isEmptyRecord(ASTContext , QualType T, bool AllowArrays);
+static bool isEmptyRecord(ASTContext , QualType T, bool AllowArrays,
+  bool AsIfNoUniqueAddr = false);
 
 /// isEmptyField - Return true iff a the field is "empty", that is it
 /// is an unnamed bit-field or an (array of) empty record(s).
 static bool isEmptyField(ASTContext , const FieldDecl *FD,
- bool AllowArrays) {
+ bool AllowArrays, bool AsIfNoUniqueAddr = false) {
   if (FD->isUnnamedBitfield())
 return true;
 
@@ -587,16 +588,19 @@
   // not arrays of records, so we must also check whether we stripped off an
   // array type above.
   if (isa(RT->getDecl()) &&
-  (WasArray || !FD->hasAttr()))
+  (WasArray || (!FD->hasAttr() && !AsIfNoUniqueAddr)))
 return false;
 
-  return isEmptyRecord(Context, FT, AllowArrays);
+  return isEmptyRecord(Context, FT, AllowArrays, AsIfNoUniqueAddr);
 }
 
 /// isEmptyRecord - Return true iff a structure contains only empty

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

2023-01-22 Thread Alex Bradbury via Phabricator via cfe-commits 
asb created this revision.
asb added reviewers: kito-cheng, jrtc27, reames, craig.topper, uweigand, luke.
Herald added subscribers: wingo, pmatos, VincentWu, vkmr, frasercrmck, evandro, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, 
shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, arichardson.
Herald added a project: All.
asb requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, MaskRay.
Herald added a project: clang.

As reported in https://github.com/llvm/llvm-project/issues/58929,
Clang's handling of empty structs in the case of small structs that may
be eligible to be passed using the hard FP calling convention doesn't
match g++. In general, C++ record fields are never empty unless
[[no_unique_address]] is used, but the RISC-V FP ABI overrides this.

After this patch, fields of structs that contain empty records will be
ignored, even in C++, when considering eligibility for the FP calling
convention ('flattening'). It isn't explicitly noted in the RISC-V
psABI, but arrays of empty records will disqualify a struct for
consideration of using the FP calling convention in g++. This patch
matches that behaviour. The psABI issue
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/358 seeks
to clarify this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142327

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/RISCV/abi-empty-structs.c

Index: clang/test/CodeGen/RISCV/abi-empty-structs.c
===
--- clang/test/CodeGen/RISCV/abi-empty-structs.c
+++ clang/test/CodeGen/RISCV/abi-empty-structs.c
@@ -29,13 +29,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local [2 x i32] @_Z7test_s12s1
-// CHECK32-CXX-SAME: ([2 x i32] [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s12s1
-// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local float @_Z7test_s12s1
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-CXX:  entry:
 //
 struct s1 test_s1(struct s1 a) {
   return a;
@@ -47,13 +43,9 @@
 // CHECK-C-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s22s2
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S2:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s22s2
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { i32, float } @_Z7test_s22s2
+// CHECK-CXX-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s2 test_s2(struct s2 a) {
   return a;
@@ -65,13 +57,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s32s3
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S3:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s32s3
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s32s3
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s3 test_s3(struct s3 a) {
   return a;
@@ -83,13 +71,9 @@
 // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
 // CHECK-C:  entry:
 //
-// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s42s4
-// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S4:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
-// CHECK32-CXX:  entry:
-//
-// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s42s4
-// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// CHECK64-CXX:  entry:
+// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s42s4
+// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-CXX:  entry:
 //
 struct s4 test_s4(struct s4 a) {
   return a;
@@ -136,6 +120,5 @@
 }
 
  NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
-// CHECK-CXX: {{.*}}
 // CHECK32-C: {{.*}}
 // CHECK64-C: {{.*}}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -548,12 +548,13 @@
   return Ctx.getOrInsertSyncScopeID(""); /* default sync scope */
 }
 
-static bool isEmptyRecord(ASTContext , QualType T,