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) {
----------------
guiand wrote:
> 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.
> nullptr_t (which has the same size and alignment as void* but is all padding)
From what I can gather looking at the IR of `test/CodeGenCXX/nullptr.cpp`, it
looks like `nullptr_t` arguments and return values are represented as normal
`i8*` types but given the special value `null`. From this it seems like we can
mark them `noundef` without worry?
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
