guiand marked an inline comment as done. guiand added inline comments.
================ Comment at: clang/lib/CodeGen/CGCall.cpp:2082 + const Type *RetTyPtr = RetTy.getTypePtr(); + if (!RetTy->isVoidType() && !RetTyPtr->isRecordType() && + RetAI.getKind() != ABIArgInfo::Indirect) { ---------------- rsmith wrote: > Other types that we should think about from a padding perspective: > > * `nullptr_t` (which has the same size and alignment as `void*` but is all > padding) > * 80-bit `long double` and `_Complex long double` (I don't know whether we > guarantee to initialize the 6 padding bytes) > * member pointers might contain padding under some ABI rules -- under the MS > ABI, you get a struct containing N pointers followed by M ints, which could > have 4 bytes of tail padding on 64-bit targets > * vector types with tail padding (eg, a vector of 3 `char`s is sometimes > passed as an `i32` with one byte `undef`) > * matrix types (presumably the same concerns as for vector types apply here) > * maybe Obj-C object types? > * `_ExtInt` types (eg, returning an `_ExtInt(65)` initialized to 0 produces > an `{i64, i64}` containing 7 bytes of `undef`) > > It would be safer to list exactly those types for which we know this > assumption is correct rather than assuming that it's correct by default -- I > think more than half of the `Type` subclasses for concrete canonical types > can have undef bits in their IR representation. This is a good point, and I think there was some nuance that I had previously included in determining this for records (since removed) that I didn't think to include in CGCall.cpp. I think one particular check, `llvm::DataLayout::typeSizeEqualsStoreSize`, handles a lot of these cases: - `long double` - oddly-sized vectors - `_ExtInt` types I don't know if the `matrix` type has internal padding (like structs) but if not it would cover that as well. I believe that the struct with padding wouldn't be an issue as we're excluding record types here. And I'd have to take a closer look at `nullptr_t` to see how it behaves here. ~~~ But if I'd like to build an exhaustive list of types I think will work correctly with a check like this: - scalars - floating point numbers - pointers I think I'd need also need an inner `typeSizeEqualsStoreSize` check on the base type of vectors/complex numbers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81678/new/ https://reviews.llvm.org/D81678 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits