[NFC] Switch FloatModeKind enum class to use bitmask enums

[jolanta.jensen]

Summary

Using bitmask enums simplifies and clarifies the code.

[aaron.ballman]

LGTM aside from a tiny naming nit. Thank you for the cleanup!

clang/include/clang/Basic/TargetInfo.h

896–897

Alternatively, you can get rid of the local variable entirely.

[jolanta.jensen]

Removed an unnecessary local variable.

[aaron.ballman]

LGTM, thanks!

[aaron.ballman]

clang/include/clang/Basic/TargetInfo.h

225–228

Whoops, I missed this (thanks @shafik for asking me about it in private)!

I believe the original code was setting the field width to be 1 << 5 bits wide instead of specifying it as being 6 bits wide.

[jolanta.jensen]

Correcting computation of RealTypeUsesObjCFPRetMask.

[jolanta.jensen]

clang/include/clang/Basic/TargetInfo.h

225–228

Oh dear, I was to replace log2 with bitWidth and I only did half the job. Thanks for spotting!

[aaron.ballman]

LGTM again, this time for real.

clang/include/clang/Basic/TargetInfo.h

225–228

Yeah, it was a great catch -- the code would have worked, but been space inefficient, which likely would have been a head scratcher for somebody later wondering why the code was written that way. :-)

[tahonermann]

@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.

clang/include/clang/Basic/TargetInfo.h

226–227

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)

896

This again directly uses the "detail" namespace. I think this would be better written as:

return (FloatModeKind)RealTypeUsesObjCFPRetMask & T;

clang/lib/Basic/Targets/X86.h

423–424

Since RealTypeUsesObjCFPRetMask is declared as unsigned, it would be best to cast to unsigned instead of int here.

703

Likewise, cast to unsigned here.

[jolanta.jensen]

@tahonermann, I addressed your comments in https://reviews.llvm.org/D129373. I just need to remove a tab that crept in and it seems to be more difficult than I thought as my code does not seem to have any tab and reformatting the code does not change the file.

clang/include/clang/Basic/TargetInfo.h

 /// Defines the clang::TargetInfo interface.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_TARGETINFO_H
 #define LLVM_CLANG_BASIC_TARGETINFO_H
 
 #include "clang/Basic/AddressSpaces.h"
+#include "clang/Basic/BitmaskEnum.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TargetCXXABI.h"
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"

 class DiagnosticsEngine;
 class LangOptions;
 class CodeGenOptions;
 class MacroBuilder;
 
 namespace Builtin { struct Info; }
 
 enum class FloatModeKind {
-  NoFloat = 255,
-  Half = 0,
-  Float,
-  Double,
-  LongDouble,
-  Float128,
-  Ibm128,
-  Last = Ibm128
+  NoFloat = 0,
+  Half = 1 << 0,
+  Float = 1 << 1,
+  Double = 1 << 2,
+  LongDouble = 1 << 3,
+  Float128 = 1 << 4,
+  Ibm128 = 1 << 5,
+  LLVM_MARK_AS_BITMASK_ENUM(Ibm128)
 };
 
 /// Fields controlling how types are laid out in memory; these may need to
 /// be copied for targets like AMDGPU that base their ABIs on an auxiliary
 /// CPU target.
 struct TransferrableTargetInfo {
   unsigned char PointerWidth, PointerAlign;
   unsigned char BoolWidth, BoolAlign;
protected:
   TargetCXXABI TheCXXABI;
   const LangASMap *AddrSpaceMap;
   unsigned ProgramAddrSpace;
 
   mutable StringRef PlatformName;
   mutable VersionTuple PlatformMinVersion;
 
   unsigned HasAlignMac68kSupport : 1;
-  unsigned RealTypeUsesObjCFPRetMask : (int)FloatModeKind::Last + 1;
+  unsigned RealTypeUsesObjCFPRetMask
+      : llvm::BitmaskEnumDetail::bitWidth(
+            (int)FloatModeKind::LLVM_BITMASK_LARGEST_ENUMERATOR);
   unsigned ComplexLongDoubleUsesFP2Ret : 1;
 
   unsigned HasBuiltinMSVaList : 1;
 
   unsigned IsRenderScriptTarget : 1;
 
   unsigned HasAArch64SVETypes : 1;
 
   unsigned HasRISCVVTypes : 1;
public:
   /// integer type enum.
   ///
   /// For example, SignedLong -> "l".
   static const char *getTypeFormatModifier(IntType T);
 
   /// Check whether the given real type should use the "fpret" flavor of
   /// Objective-C message passing on this target.
   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);
   }
 
   /// Check whether _Complex long double should use the "fp2ret" flavor
   /// of Objective-C message passing on this target.
   bool useObjCFP2RetForComplexLongDouble() const {
     return ComplexLongDoubleUsesFP2Ret;
   }
 
   /// Check whether llvm intrinsics such as llvm.convert.to.fp16 should be used

clang/lib/Basic/Targets/X86.h

 //===--- X86.h - Declare X86 target feature support -------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file declares X86 TargetInfo objects.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_LIB_BASIC_TARGETS_X86_H
 #define LLVM_CLANG_LIB_BASIC_TARGETS_X86_H
 
 #include "OSTargets.h"
+#include "clang/Basic/BitmaskEnum.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/X86TargetParser.h"
 
 namespace clang {
 namespace targets {
resetDataLayout(
         Triple.isOSBinFormatMachO() ? "_" : "");
     SizeType = UnsignedInt;
     PtrDiffType = SignedInt;
     IntPtrType = SignedInt;
     RegParmMax = 3;
 
     // Use fpret for all types.
     RealTypeUsesObjCFPRetMask =
-        ((1 << (int)FloatModeKind::Float) | (1 << (int)FloatModeKind::Double) |
-         (1 << (int)FloatModeKind::LongDouble));
+        (int)(FloatModeKind::Float | FloatModeKind::Double |
+              FloatModeKind::LongDouble);
 
     // x86-32 has atomics up to 8 bytes
     MaxAtomicPromoteWidth = 64;
     MaxAtomicInlineWidth = 32;
   }
 
   BuiltinVaListKind getBuiltinVaListKind() const override {
     return TargetInfo::CharPtrBuiltinVaList;
X86_64TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
     resetDataLayout(IsX32 ? "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-"
                             "i64:64-f80:128-n8:16:32:64-S128"
                           : IsWinCOFF ? "e-m:w-p270:32:32-p271:32:32-p272:64:"
                                         "64-i64:64-f80:128-n8:16:32:64-S128"
                                       : "e-m:e-p270:32:32-p271:32:32-p272:64:"
                                         "64-i64:64-f80:128-n8:16:32:64-S128");
 
     // Use fpret only for long double.
-    RealTypeUsesObjCFPRetMask = (1 << (int)FloatModeKind::LongDouble);
+    RealTypeUsesObjCFPRetMask = (int)FloatModeKind::LongDouble;
 
     // Use fp2ret for _Complex long double.
     ComplexLongDoubleUsesFP2Ret = true;
 
     // Make __builtin_ms_va_list available.
     HasBuiltinMSVaList = true;
 
     // x86-64 has atomics up to 16 bytes.