[PATCH] D41327: [X86] Refactor DomainReassignment pass to make the Closure class not stores references to the main data structures of the pass itself
Guy Blank via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 18 00:13:59 PST 2017
guyblank added inline comments.
================
Comment at: lib/Target/X86/X86DomainReassignment.cpp:311
/// Domains which this closure can legally be reassigned to.
std::bitset<NumDomains> LegalDstDomains;
----------------
craig.topper wrote:
> guyblank wrote:
> > i don't see this in top of trunk, is this the right diff?
> Should have been committed in r320940.
yes, sorry.
================
Comment at: lib/Target/X86/X86DomainReassignment.cpp:317
- /// The register domain of this closure.
- RegDomain Domain;
-
----------------
IMO the domain should remain part of the closure itself. as opposed to the other removed fields, it really is a property of the closure.
================
Comment at: lib/Target/X86/X86DomainReassignment.cpp:328
+ /// Mark this close sure as illegal for reassignment to domain \p RD.
+ void setIllegal(RegDomain RD) { LegalDstDomains[RD] = false; }
----------------
typo
close sure -> closure
================
Comment at: lib/Target/X86/X86DomainReassignment.cpp:338
+ using const_edge_iterator = DenseSet<unsigned>::const_iterator;
+ iterator_range<const_edge_iterator> edges() {
+ return iterator_range<const_edge_iterator>(Edges.begin(), Edges.end());
----------------
the function can be const
================
Comment at: lib/Target/X86/X86DomainReassignment.cpp:396
+ /// Reassign the closure to \p Domain.
+ void Reassign(Closure &C, RegDomain Domain) const;
+
----------------
Reassign -> reassign
https://reviews.llvm.org/D41327
More information about the llvm-commits
mailing list