aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/TargetInfo.h:885
bool useObjCFPRetForRealType(FloatModeKind T) const {
+ assert(T <= FloatModeKind::Last);
return RealTypeUsesObjCFPRet & (1 << (int)T);
----------------
This will assert if passed `NoFloat` -- is that intentional?
You should also add `&& "some helpful message"` to the assert predicate so that
when the assert fails there's some more information about why the failure
happened.
================
Comment at: clang/include/clang/Basic/TargetInfo.h:223-224
unsigned HasAlignMac68kSupport : 1;
- unsigned RealTypeUsesObjCFPRet : 3;
+ unsigned RealTypeUsesObjCFPRet : (1 << (int)FloatModeKind::Float) |
+ (1 << (int)FloatModeKind::Double);
unsigned ComplexLongDoubleUsesFP2Ret : 1;
----------------
jolanta.jensen wrote:
> tahonermann wrote:
> > This doesn't look right to me. The size of the bit field would be `(1 << 1)
> > | (1 << 2)` which is `0b110` which is 6, but more by coincidence than by
> > construction. I think what we want is:
> > unsigned RealTypeUsesObjCFPRet : (int)FloatModeKind::Last + 1;
> Sorry. I mixed things up.
>I think what we want is:
>`unsigned RealTypeUsesObjCFPRet : (int)FloatModeKind::Last + 1;`
I think this is wrong in two different ways. We need as many bits as required
to store the enumerator value. The highest value is 255 (`NoFloat`), so we need
at least 8 bits for that. But also, the last enumerator value is just that -- a
value, not a width.
I'd probably skip the `Last` entry and force the bit-field to 8 bits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126479/new/
https://reviews.llvm.org/D126479
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
