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

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

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

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 ___

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

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

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

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

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

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;

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

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

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

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: > >

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

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)) +

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

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

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

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

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

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.

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

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

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

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

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

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!).

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

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/

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

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/

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

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() &&

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

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

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

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,