[clang] [X86_32][C++] fix 0 sized struct case in vaarg. (PR #86388)
https://github.com/CoTinker edited https://github.com/llvm/llvm-project/pull/86388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86_32][C++] fix 0 sized struct case in vaarg. (PR #86388)
efriedma-quic wrote: If you implement #86075 like I suggested, the inconsistency here also goes away, I think: if va_arg queries classifyArgumentType, you get the same result as argument lowering, so clang becomes self-consistent. (Whether that's gcc-compatible is a different question...) https://github.com/llvm/llvm-project/pull/86388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86_32][C++] fix 0 sized struct case in vaarg. (PR #86388)
CoTinker wrote: Inconsistency here is inevitable. Such struct `SuperEmpty { struct{ int a[0];} b;}` in c++ mode can not be ignored in i386 for that c++ fields are never empty, and its TypeInfo.Width = TypeInfo.Align = 0. https://github.com/llvm/llvm-project/blob/594989918bf9e178f3b7c5e200d700d0574f15ec/clang/lib/CodeGen/Targets/X86.cpp#L1070-L1091 And Indirect is false, so that DirectSize = DirectAlign = 0, which leads to error. https://github.com/llvm/llvm-project/blob/594989918bf9e178f3b7c5e200d700d0574f15ec/clang/lib/CodeGen/ABIInfoImpl.cpp#L205-L230 How do I fix this? Change the classifyArgumentType or the EmiVAArg? @efriedma-quic https://github.com/llvm/llvm-project/pull/86388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86_32][C++] fix 0 sized struct case in vaarg. (PR #86388)
efriedma-quic wrote: There are two ways we could analyze, this, I think: - EmitVAArg is doing something which disagrees with the classification done by classifyArgumentType, so that's a bug in EmitVAArg. - classifyArgumentType is returning a malformed value; EmitVAArg should assert that it never sees such a value, and classifyArgumentType should be changed to return something else in that case. What exactly is classifyArgumentType returning here? https://github.com/llvm/llvm-project/pull/86388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86_32][C++] fix 0 sized struct case in vaarg. (PR #86388)
https://github.com/CoTinker updated https://github.com/llvm/llvm-project/pull/86388 >From 54f75d7068d0acf336d6d1e61bcf800ff52cc310 Mon Sep 17 00:00:00 2001 From: Longsheng Mou Date: Sat, 23 Mar 2024 17:53:58 +0800 Subject: [PATCH] [X86_32] fix 0 sized struct case in vaarg. struct SuperEmpty { struct{ int a[0];} b;}; Such 0 sized structs in c++ mode can not be ignored in i386 for that c++ fields are never empty.But when EmitVAArg, its size is 0, so that va_list not increase.Maybe we can ignore this kind of arguments. --- clang/lib/CodeGen/Targets/X86.cpp | 4 clang/test/CodeGenCXX/regparm.cpp | 2 +- clang/test/CodeGenCXX/x86_32-vaarg.cpp | 22 ++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp index 16d52bee3490b..26ff4e4ac0a3b 100644 --- a/clang/lib/CodeGen/Targets/X86.cpp +++ b/clang/lib/CodeGen/Targets/X86.cpp @@ -795,6 +795,10 @@ ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty, CCState &State, if (!IsWin32StructABI && isEmptyRecord(getContext(), Ty, true)) return ABIArgInfo::getIgnore(); +// Ignore 0 sized structs. +if (TI.Width == 0) + return ABIArgInfo::getIgnore(); + llvm::LLVMContext &LLVMContext = getVMContext(); llvm::IntegerType *Int32 = llvm::Type::getInt32Ty(LLVMContext); bool NeedsPadding = false; diff --git a/clang/test/CodeGenCXX/regparm.cpp b/clang/test/CodeGenCXX/regparm.cpp index 1fd471c2d0727..b9735485db8de 100644 --- a/clang/test/CodeGenCXX/regparm.cpp +++ b/clang/test/CodeGenCXX/regparm.cpp @@ -32,7 +32,7 @@ struct S3 { } a; }; __attribute((regparm(2))) void foo4(S3 a, int b); -// CHECK: declare void @_Z4foo42S3i(ptr noundef byval(%struct.S3) align 4, i32 inreg noundef) +// CHECK: declare void @_Z4foo42S3i(i32 inreg noundef) void bar3(S3 a, int b) { foo4(a, b); } diff --git a/clang/test/CodeGenCXX/x86_32-vaarg.cpp b/clang/test/CodeGenCXX/x86_32-vaarg.cpp index a3f2791484362..90665652a0286 100644 --- a/clang/test/CodeGenCXX/x86_32-vaarg.cpp +++ b/clang/test/CodeGenCXX/x86_32-vaarg.cpp @@ -18,3 +18,25 @@ empty empty_record_test(int z, ...) { __builtin_va_start(list, z); return __builtin_va_arg(list, empty); } + +typedef struct { + struct { +int a[0]; + } b; +} SortOfEmpty; + +// CHECK-LABEL: @_Z18test_sort_of_emptyiz( +// CHECK-NEXT: entry: +// CHECK-NEXT:[[RESULT_PTR:%.*]] = alloca ptr, align 4 +// CHECK-NEXT:[[Z_ADDR:%.*]] = alloca i32, align 4 +// CHECK-NEXT:[[LIST:%.*]] = alloca ptr, align 4 +// CHECK-NEXT:store ptr [[AGG_RESULT:%.*]], ptr [[RESULT_PTR]], align 4 +// CHECK-NEXT:store i32 [[Z:%.*]], ptr [[Z_ADDR]], align 4 +// CHECK-NEXT:call void @llvm.va_start.p0(ptr [[LIST]]) +// CHECK-NEXT:ret void +// +SortOfEmpty test_sort_of_empty(int z, ...) { + __builtin_va_list list; + __builtin_va_start(list, z); + return __builtin_va_arg(list, SortOfEmpty); +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86_32][C++] fix 0 sized struct case in vaarg. (PR #86388)
https://github.com/CoTinker edited https://github.com/llvm/llvm-project/pull/86388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86_32][C++] fix 0 sized struct case in vaarg. (PR #86388)
CoTinker wrote: Could you please take time to review it, thank you. @efriedma-quic https://github.com/llvm/llvm-project/pull/86388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86_32][C++] fix 0 sized struct case in vaarg. (PR #86388)
https://github.com/efriedma-quic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/86388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86_32][C++] fix 0 sized struct case in vaarg. (PR #86388)
https://github.com/CoTinker closed https://github.com/llvm/llvm-project/pull/86388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86_32][C++] fix 0 sized struct case in vaarg. (PR #86388)
RSilicon wrote: Should this be backported to 19.x? https://github.com/llvm/llvm-project/pull/86388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86_32][C++] fix 0 sized struct case in vaarg. (PR #86388)
CoTinker wrote: I don't know, but maybe Eli can give you an answer. https://github.com/llvm/llvm-project/pull/86388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits