rjmccall added a comment.
I'm fine with proceeding with your best guess about what the ABI should be.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:9232
+ if (IsFloat && Size > FLen)
+ return false;
+ // Can't be eligible if an integer type was already found (only fp+int or
----------------
asb wrote:
> rjmccall wrote:
> > Is this the only consideration for floating-point types? Clang does have
> > increasing support for `half` / various `float16` types.
> These types aren't supported on RISC-V currently. As the ABI hasn't really
> been explicitly confirmed, I've defaulted to the integer ABI in that case.
> Could move to an assert if you prefer, though obviously any future move to
> enable half floats for RISC-V should include ABI tests too.
Defaulting to the integer ABI is fine.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:9236
+ if (IsInt && Field1Ty && Field1Ty->isIntegerTy())
+ return false;
+ if (!Field1Ty) {
----------------
asb wrote:
> rjmccall wrote:
> > The comment here is wrong because fp+fp is allowed, right?
> >
> > Is this not already caught by the post-processing checks you do in
> > `detectFPCCEligibleStruct`? Would it make more sense to just do all those
> > checks there?
> Thanks, I meant to say int+int isn't eligible. Reworded to say that.
>
> I don't think it would simplify things to do all checks in
> detectFPCCEligibleStruct. More repetition would be required in order to do
> checks on both Float1Ty and Float2Ty.
Okay. It just seemed to me that responsibility was oddly split between the
functions.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:9340
+ if (getContext().getTypeSize(QTy) > XLen && BitWidth <= XLen)
+ QTy = getContext().getIntTypeForBitwidth(XLen, false);
+ if (BitWidth == 0) {
----------------
Okay. So consecutive bit-fields are considered individually, not packed into a
single storage unit and then considered? Unfortunate ABI rule, but if it's
what you have to implement, so be it.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60456/new/
https://reviews.llvm.org/D60456
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
