[PATCH] D76170: [OpenMP][NFC] Reduce instantiation time with different data structure

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Thanks, yes, this is what I had in mind.



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPContext.h:48
 #include "llvm/Frontend/OpenMP/OMPKinds.def"
+NUM_PROPERTIES
 };
----------------
LLVM doesn't use the all uppercase naming style for enums, I'd use something like Count, Num, or Max.

One technique you could use to avoid the sentinel enum is to make an alias for the last enum, as is done for builtin types here:
https://github.com/llvm/llvm-project/blob/master/clang/include/clang/AST/Type.h#L2514
Then you won't have to handle NUM_PROPERTIES below, and you would use `TraitProperty::Max + 1` for the BitVector.


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPContext.h:152
 
   SmallSet<TraitProperty, 8> ActiveTraits;
   SmallVector<TraitProperty, 8> ConstructTraits;
----------------
This is the same instantiation, so we don't get any savings unless the same technique is applied here. I started by trying to change this type, but it was challenging, so I raised the issue to see if you'd be OK with it.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPContext.cpp:129
+    if (!VMI1.RequiredTraits.test(SetBitsIt))
+        return false;
+  if (!isSubset<TraitProperty>(VMI0.ConstructTraits, VMI1.ConstructTraits))
----------------
Worth fixing this formatting nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76170