Page MenuHomePhabricator

[OpenMP][NFC] Minimize memory usage and copying of `OMPTraitInfo`s
ClosedPublic

Authored by jdoerfert on Fri, Mar 13, 9:50 PM.

Diff Detail

Event Timeline

jdoerfert created this revision.Fri, Mar 13, 9:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptFri, Mar 13, 9:50 PM
rnk accepted this revision.Sat, Mar 14, 8:43 AM

lgtm, thanks!

I also noticed that Parser.h includes OpenMPClause.h just for this class. OpenMPClause.h is pretty big. It's for this family of related methods:

/// Parse a property kind into \p TIProperty for the selector set \p Set and
/// selector \p Selector.
void parseOMPTraitPropertyKind(OMPTraitInfo::OMPTraitProperty &TIProperty,
                               llvm::omp::TraitSet Set,
                               llvm::omp::TraitSelector Selector,
                               llvm::StringMap<SourceLocation> &Seen);

The use of an inner class here makes it impossible to forward declare OMPTraitProperty. Would you be opposed to moving it out of line?

The other popular includer of OpenMPClause.h is Attr.h, which is where I started my investigation. I think after your change, we can forward declare it. I'll look into it.

This revision is now accepted and ready to land.Sat, Mar 14, 8:43 AM
In D76173#1922916, @rnk wrote:

lgtm, thanks!

I also noticed that Parser.h includes OpenMPClause.h just for this class. OpenMPClause.h is pretty big. It's for this family of related methods:

/// Parse a property kind into \p TIProperty for the selector set \p Set and
/// selector \p Selector.
void parseOMPTraitPropertyKind(OMPTraitInfo::OMPTraitProperty &TIProperty,
                               llvm::omp::TraitSet Set,
                               llvm::omp::TraitSelector Selector,
                               llvm::StringMap<SourceLocation> &Seen);

The use of an inner class here makes it impossible to forward declare OMPTraitProperty. Would you be opposed to moving it out of line?

Like D76184 ?

The other popular includer of OpenMPClause.h is Attr.h, which is where I started my investigation. I think after your change, we can forward declare it. I'll look into it.

Took care of that in D76184 as well. I will "soonish" redesign OpenMPClause.h completely but that will take more time.

This revision was automatically updated to reflect the committed changes.