tahonermann added a comment.
@jolanta.jensen, I was on vacation last week and missed my opportunity to
comment on this. I know the change has already landed, but I added a few
comments anyway.
================
Comment at: clang/include/clang/Basic/TargetInfo.h:226-227
+ unsigned RealTypeUsesObjCFPRetMask
+ : llvm::BitmaskEnumDetail::bitWidth(
+ (int)FloatModeKind::LLVM_BITMASK_LARGEST_ENUMERATOR);
unsigned ComplexLongDoubleUsesFP2Ret : 1;
----------------
I believe this can be simplified to:
: llvm::BitWidth<FloatModeKind>;
(It should never be necessary to reach directly into a "detail" namespace from
user code)
================
Comment at: clang/include/clang/Basic/TargetInfo.h:896
bool useObjCFPRetForRealType(FloatModeKind T) const {
- assert(T <= FloatModeKind::Last &&
- "T value is larger than RealTypeUsesObjCFPRetMask can handle");
- return RealTypeUsesObjCFPRetMask & (1 << (int)T);
+ return RealTypeUsesObjCFPRetMask & llvm::BitmaskEnumDetail::Underlying(T);
}
----------------
This again directly uses the "detail" namespace. I think this would be better
written as:
return (FloatModeKind)RealTypeUsesObjCFPRetMask & T;
================
Comment at: clang/lib/Basic/Targets/X86.h:423-424
RealTypeUsesObjCFPRetMask =
- ((1 << (int)FloatModeKind::Float) | (1 << (int)FloatModeKind::Double) |
- (1 << (int)FloatModeKind::LongDouble));
+ (int)(FloatModeKind::Float | FloatModeKind::Double |
+ FloatModeKind::LongDouble);
----------------
Since `RealTypeUsesObjCFPRetMask` is declared as `unsigned`, it would be best
to cast to `unsigned` instead of `int` here.
================
Comment at: clang/lib/Basic/Targets/X86.h:703
// Use fpret only for long double.
- RealTypeUsesObjCFPRetMask = (1 << (int)FloatModeKind::LongDouble);
+ RealTypeUsesObjCFPRetMask = (int)FloatModeKind::LongDouble;
----------------
Likewise, cast to `unsigned` here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128182/new/
https://reviews.llvm.org/D128182
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
