TIFitis marked 2 inline comments as done. TIFitis added inline comments.
================ Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:193 - OMP_TGT_EXEC_MODE_GENERIC | OMP_TGT_EXEC_MODE_SPMD, - LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue */ OMP_TGT_EXEC_MODE_GENERIC_SPMD) }; ---------------- jdoerfert wrote: > TIFitis wrote: > > jdoerfert wrote: > > > TIFitis wrote: > > > > I am not sure if this change is safe. It can be avoided by making > > > > `OpenMPOffloadMappingFlags` an enum class. > > > Why do you need to change this enum at all? > > Otherwise you'd have two declarations of LLVM_MARK_AS_BITMASK_ENUM in the > > same namespace which is an error ofc. > > > > This is because they are both enums which spill the declarations to the > > enclosing namespace, I.e. llvm::omp > But we are using that type in binary operations w/o cast. Does this change > then not break existing code? Going by the comments `LLVM_MARK_AS_BITMASK_ENUM` should be used to mark the largest individual enum, don't see any restrictions on it being used with multiple enum's in the same namespace. `check-all` is also clean. I don't see any evidence of this being unsafe. Changing `OpenMPOffloadMappingFlags` to enum class however, would be the safest thing to do but that would introduce a lot of ugly static_cast everywhere. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140292/new/ https://reviews.llvm.org/D140292 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
