[PATCH] D59002: Use bitset for assembler predicates

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec marked 3 inline comments as done.
rampitec added inline comments.


================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:10484
       // Don't report the same set of features twice.
-      if (FeatureMissesSeen.count(MissingFeatures))
+      if (FeatureMissesSeen.count(MissingFeatures.to_ullong()))
         break;
----------------
rampitec wrote:
> craig.topper wrote:
> > This seems to be making the assumption that ARM feature bits fit in a unisgned long long?
> Yes, if exceptions enabled it would raise an exception. The set cannot be constructed from a FeatureBitset since it does not have comparison operators.
> I would add an assert, but unfortunately cannot enumerate all features. The solution can be to use a SmallVector, but as long as it fits 64 bit this seems to be an overkill.
I have just added comparison operator to be on a safe side.


================
Comment at: utils/TableGen/AsmMatcherEmitter.cpp:3392
                << " ConvertFn;\n";
-  OS << "    " << getMinimalTypeForEnumBitfield(Info.SubtargetFeatures.size())
-               << " RequiredFeatures;\n";
+  OS << "    " << "unsigned RequiredFeaturesIdx;\n";
   OS << "    " << getMinimalTypeForRange(
----------------
craig.topper wrote:
> Surely we can bound this to less than a full unsigned? X86 uses almost no features in the assembler which means we can probably use a uint8_t and MatchEntry is used about 28000 times. So that's going to save like 3 * 28000 bytes just from X86.
BTW, table is now smaller. It used to be full 64 bit before per record. OperandMatchEntry can also be rearranged to save on padding now.


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

https://reviews.llvm.org/D59002