[PATCH] D59002: Use bitset for assembler predicates

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: include/llvm/MC/MCParser/MCTargetAsmParser.h:258
   // not have.
-  uint64_t getFeatures() const {
+  FeatureBitset getFeatures() const {
     assert(Kind == NearMissFeature);
----------------
return a reference


================
Comment at: include/llvm/MC/MCParser/MCTargetAsmParser.h:363
 
-  uint64_t getAvailableFeatures() const { return AvailableFeatures; }
-  void setAvailableFeatures(uint64_t Value) { AvailableFeatures = Value; }
+  FeatureBitset getAvailableFeatures() const { return AvailableFeatures; }
+  void setAvailableFeatures(FeatureBitset Value) { AvailableFeatures = Value; }
----------------
Should this return a "const FeatureBitset&"?


================
Comment at: include/llvm/MC/MCParser/MCTargetAsmParser.h:364
+  FeatureBitset getAvailableFeatures() const { return AvailableFeatures; }
+  void setAvailableFeatures(FeatureBitset Value) { AvailableFeatures = Value; }
 
----------------
Should probably take "const FeatureBitset&"


================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:4169
 
-static std::string AArch64MnemonicSpellCheck(StringRef S, uint64_t FBS,
+static std::string AArch64MnemonicSpellCheck(StringRef S, FeatureBitset FBS,
                                              unsigned VariantID = 0);
----------------
Probably should pass the bits by const reference?


================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp:192
   void verifyInstructionPredicates(const MCInst &MI,
-                                   uint64_t AvailableFeatures) const;
+                                   FeatureBitset AvailableFeatures) const;
 };
----------------
const FeatureBitset&


================
Comment at: lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.h:69
   void verifyInstructionPredicates(const MCInst &MI,
-                                   uint64_t AvailableFeatures) const;
+                                   FeatureBitset AvailableFeatures) const;
 };
----------------
const FeatureBitset&


================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:6070
   // any sort of suffix.
-  uint64_t AvailableFeatures = getAvailableFeatures();
+  FeatureBitset AvailableFeatures = getAvailableFeatures();
   unsigned AssemblerDialect = getParser().getAssemblerDialect();
----------------
const FeatureBitset&


================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:10481
     case NearMissInfo::NearMissFeature: {
-      uint64_t MissingFeatures = I.getFeatures();
+      FeatureBitset MissingFeatures = I.getFeatures();
       // Don't report the same set of features twice.
----------------
Can be a const reference?


================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:10598
 } Extensions[] = {
-  { ARM::AEK_CRC, Feature_HasV8, {ARM::FeatureCRC} },
-  { ARM::AEK_CRYPTO,  Feature_HasV8,
+  { ARM::AEK_CRC, {Feature_HasV8Bit}, {ARM::FeatureCRC} },
+  { ARM::AEK_CRYPTO,  {Feature_HasV8Bit},
----------------
This invokes the std::initializer_list constructor in FeatureBitset that can't be evaluated at compile time. Thus this is calling a static global constructor.

I just recently fixed this issue in the *GenSubtargetInfo.inc files


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59002/new/

https://reviews.llvm.org/D59002