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