zoecarver added inline comments.
================ Comment at: clang/lib/Sema/SemaDeclCXX.cpp:6502 + // except that it has a non-trivial member *with* the trivial_abi attribute. + for (auto Base : D->bases()) { + if (auto CxxRecord = Base.getType()->getAsCXXRecordDecl()) ---------------- ahatanak wrote: > zoecarver wrote: > > ahatanak wrote: > > > It looks like this patch changes the way `D` is passed in the following > > > code: > > > > > > ``` > > > struct B { > > > int i[4]; > > > B(); > > > B(const B &) = default; > > > B(B &&); > > > }; > > > > > > struct D : B { > > > D(); > > > D(const D &) = default; > > > D(D &&) = delete; > > > }; > > > > > > void testB(B a); > > > void testD(D a); > > > > > > void testCallB() { > > > B b; > > > testB(b); > > > } > > > > > > void testCallD() { > > > D d; > > > testD(d); > > > } > > > ``` > > > > > > `B` cannot be passed in registers because it has a non-trivial move > > > constructor, whereas `D` can be passed in registers because the move > > > constructor is deleted and the copy constructor is trivial. > > > > > > I'm not sure what the best way to handle this is, but I just wanted to > > > point this out. > > Hmm. Good catch. One way to fix this would be to simply create a > > `HasPassableSubobject` variable and add that to the conditions below > > (instead of returning false here). But, it seems that `D` isn't passed by > > registers (even though, maybe it should be) on ToT: > > https://godbolt.org/z/4xevW5 > > > > Given that, do you think it's OK to return false here, or should I update > > this patch to use the logic I just described (even though that would be a > > nfc)? > The argument is byval, so `D` is passed directly. If you remove `-O3` and add > `-target aarch64`, you'll see that `[2 x i64]` is being passed Ah, I see now. Great. Thanks. I'll update the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92361/new/ https://reviews.llvm.org/D92361 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits