ABataev added inline comments.
================ Comment at: lib/CodeGen/CGOpenMPRuntime.h:2110 + /// Emit code for the user defined mapper construct. + void emitUserDefinedMapper(const OMPDeclareMapperDecl *D, + CodeGenFunction *CGF = nullptr) override; ---------------- lildmh wrote: > ABataev wrote: > > lildmh wrote: > > > lildmh wrote: > > > > ABataev wrote: > > > > > ABataev wrote: > > > > > > lildmh wrote: > > > > > > > ABataev wrote: > > > > > > > > I think you can drop this function here if the original > > > > > > > > function is not virtual > > > > > > > The function for simd only mode includes a `llvm_unreachable`, so > > > > > > > I think it's still needed as a virtual function above > > > > > > It is better to reduce number of virtual functions, if possible. > > > > > What about virtual function? > > > > Since `emitUserDefinedMapper` here overrides the same function in > > > > `CGOpenMPRuntime`, I think `emitUserDefinedMapper` of `CGOpenMPRuntime` > > > > needs to be defined as `virtual`? > > > If you are asking about what virtual function I removed, it is > > > `emitUDMapperArrayInitOrDel` of `CGOpenMPRuntime` which is never > > > overrided. > > I would suggest to not make this virtual too and remove overriden version. > > It does not help. > If we remove virtual from `emitUserDefinedMapper`, we will have to define it > in both `CGOpenMPRuntime` and `CGOpenMPSIMDRuntime`, and its definition is > identical in both places. I don't think that's good software engineering > practice? WE just don't need an overridden version in `CGOpenMPSIMDRuntime`. Just remove virtual and remove the overridden function from `CGOpenMPSIMDRuntime`. You have the check for `OpenMPSimd` mode that prevents the emission of mappers in simd-only mode. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59474/new/ https://reviews.llvm.org/D59474 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
