[llvm] r190793 - TableGen: give asm match classes deterministic order.
Howard Hinnant
hhinnant at apple.com
Mon Sep 16 10:27:33 PDT 2013
It would make my day if you could make the LessRegisterSet operator() const:
struct LessRegisterSet {
bool operator() (const RegisterSet &LHS, const RegisterSet & RHS) { // <----- needs to be const
// std::set<T> defines its own compariso "operator<", but it
// performs a lexicographical comparison by T's innate comparison
// for some reason. We don't want non-deterministic pointer
// comparisons so use this instead.
return std::lexicographical_compare(LHS.begin(), LHS.end(),
RHS.begin(), RHS.end(),
LessRecordByID());
}
};
Thanks,
Howard
On Sep 16, 2013, at 12:43 PM, Tim Northover <tnorthover at apple.com> wrote:
> Author: tnorthover
> Date: Mon Sep 16 11:43:19 2013
> New Revision: 190793
>
> URL: http://llvm.org/viewvc/llvm-project?rev=190793&view=rev
> Log:
> TableGen: give asm match classes deterministic order.
>
> TableGen was sorting the entries in some of its internal data
> structures by pointer. This order filtered through to the final
> matching table and affected the diagnostics produced on bad assembly
> occasionally.
>
> It also turns out STL algorithms are ridiculously easy to misuse on
> containers with custom order methods. (No bugs before, or now that I
> know of, but plenty in the middle).
>
> This should fix the sanitizer bot, which ends up with weird pointers.
>
> Modified:
> llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp
>
> Modified: llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp?rev=190793&r1=190792&r2=190793&view=diff
> ==============================================================================
> --- llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp Mon Sep 16 11:43:19 2013
> @@ -125,6 +125,13 @@ namespace {
> class AsmMatcherInfo;
> struct SubtargetFeatureInfo;
>
> +// Register sets are used as keys in some second-order sets TableGen creates
> +// when generating its data structures. This means that the order of two
> +// RegisterSets can be seen in the outputted AsmMatcher tables occasionally, and
> +// can even affect compiler output (at least seen in diagnostics produced when
> +// all matches fail). So we use a type that sorts them consistently.
> +typedef std::set<Record*, LessRecordByID> RegisterSet;
> +
> class AsmMatcherEmitter {
> RecordKeeper &Records;
> public:
> @@ -185,7 +192,7 @@ struct ClassInfo {
> std::string ParserMethod;
>
> /// For register classes, the records for all the registers in this class.
> - std::set<Record*> Registers;
> + RegisterSet Registers;
>
> /// For custom match classes, he diagnostic kind for when the predicate fails.
> std::string DiagnosticType;
> @@ -213,11 +220,11 @@ public:
> if (!isRegisterClass() || !RHS.isRegisterClass())
> return false;
>
> - std::set<Record*> Tmp;
> - std::insert_iterator< std::set<Record*> > II(Tmp, Tmp.begin());
> + RegisterSet Tmp;
> + std::insert_iterator<RegisterSet> II(Tmp, Tmp.begin());
> std::set_intersection(Registers.begin(), Registers.end(),
> RHS.Registers.begin(), RHS.Registers.end(),
> - II);
> + II, LessRecordByID());
>
> return !Tmp.empty();
> }
> @@ -1057,6 +1064,18 @@ AsmMatcherInfo::getOperandClass(Record *
> PrintFatalError(Rec->getLoc(), "operand has no match class!");
> }
>
> +struct LessRegisterSet {
> + bool operator() (const RegisterSet &LHS, const RegisterSet & RHS) {
> + // std::set<T> defines its own compariso "operator<", but it
> + // performs a lexicographical comparison by T's innate comparison
> + // for some reason. We don't want non-deterministic pointer
> + // comparisons so use this instead.
> + return std::lexicographical_compare(LHS.begin(), LHS.end(),
> + RHS.begin(), RHS.end(),
> + LessRecordByID());
> + }
> +};
> +
> void AsmMatcherInfo::
> buildRegisterClasses(SmallPtrSet<Record*, 16> &SingletonRegisters) {
> const std::vector<CodeGenRegister*> &Registers =
> @@ -1064,33 +1083,35 @@ buildRegisterClasses(SmallPtrSet<Record*
> ArrayRef<CodeGenRegisterClass*> RegClassList =
> Target.getRegBank().getRegClasses();
>
> + typedef std::set<RegisterSet, LessRegisterSet> RegisterSetSet;
> +
> // The register sets used for matching.
> - std::set< std::set<Record*> > RegisterSets;
> + RegisterSetSet RegisterSets;
>
> // Gather the defined sets.
> for (ArrayRef<CodeGenRegisterClass*>::const_iterator it =
> - RegClassList.begin(), ie = RegClassList.end(); it != ie; ++it)
> - RegisterSets.insert(std::set<Record*>(
> + RegClassList.begin(), ie = RegClassList.end(); it != ie; ++it)
> + RegisterSets.insert(RegisterSet(
> (*it)->getOrder().begin(), (*it)->getOrder().end()));
>
> // Add any required singleton sets.
> for (SmallPtrSet<Record*, 16>::iterator it = SingletonRegisters.begin(),
> ie = SingletonRegisters.end(); it != ie; ++it) {
> Record *Rec = *it;
> - RegisterSets.insert(std::set<Record*>(&Rec, &Rec + 1));
> + RegisterSets.insert(RegisterSet(&Rec, &Rec + 1));
> }
>
> // Introduce derived sets where necessary (when a register does not determine
> // a unique register set class), and build the mapping of registers to the set
> // they should classify to.
> - std::map<Record*, std::set<Record*> > RegisterMap;
> + std::map<Record*, RegisterSet> RegisterMap;
> for (std::vector<CodeGenRegister*>::const_iterator it = Registers.begin(),
> ie = Registers.end(); it != ie; ++it) {
> const CodeGenRegister &CGR = **it;
> // Compute the intersection of all sets containing this register.
> - std::set<Record*> ContainingSet;
> + RegisterSet ContainingSet;
>
> - for (std::set< std::set<Record*> >::iterator it = RegisterSets.begin(),
> + for (RegisterSetSet::iterator it = RegisterSets.begin(),
> ie = RegisterSets.end(); it != ie; ++it) {
> if (!it->count(CGR.TheDef))
> continue;
> @@ -1100,11 +1121,12 @@ buildRegisterClasses(SmallPtrSet<Record*
> continue;
> }
>
> - std::set<Record*> Tmp;
> + RegisterSet Tmp;
> std::swap(Tmp, ContainingSet);
> - std::insert_iterator< std::set<Record*> > II(ContainingSet,
> - ContainingSet.begin());
> - std::set_intersection(Tmp.begin(), Tmp.end(), it->begin(), it->end(), II);
> + std::insert_iterator<RegisterSet> II(ContainingSet,
> + ContainingSet.begin());
> + std::set_intersection(Tmp.begin(), Tmp.end(), it->begin(), it->end(), II,
> + LessRecordByID());
> }
>
> if (!ContainingSet.empty()) {
> @@ -1114,9 +1136,9 @@ buildRegisterClasses(SmallPtrSet<Record*
> }
>
> // Construct the register classes.
> - std::map<std::set<Record*>, ClassInfo*> RegisterSetClasses;
> + std::map<RegisterSet, ClassInfo*, LessRegisterSet> RegisterSetClasses;
> unsigned Index = 0;
> - for (std::set< std::set<Record*> >::iterator it = RegisterSets.begin(),
> + for (RegisterSetSet::iterator it = RegisterSets.begin(),
> ie = RegisterSets.end(); it != ie; ++it, ++Index) {
> ClassInfo *CI = new ClassInfo();
> CI->Kind = ClassInfo::RegisterClass0 + Index;
> @@ -1134,13 +1156,14 @@ buildRegisterClasses(SmallPtrSet<Record*
>
> // Find the superclasses; we could compute only the subgroup lattice edges,
> // but there isn't really a point.
> - for (std::set< std::set<Record*> >::iterator it = RegisterSets.begin(),
> + for (RegisterSetSet::iterator it = RegisterSets.begin(),
> ie = RegisterSets.end(); it != ie; ++it) {
> ClassInfo *CI = RegisterSetClasses[*it];
> - for (std::set< std::set<Record*> >::iterator it2 = RegisterSets.begin(),
> + for (RegisterSetSet::iterator it2 = RegisterSets.begin(),
> ie2 = RegisterSets.end(); it2 != ie2; ++it2)
> if (*it != *it2 &&
> - std::includes(it2->begin(), it2->end(), it->begin(), it->end()))
> + std::includes(it2->begin(), it2->end(), it->begin(), it->end(),
> + LessRecordByID()))
> CI->SuperClasses.push_back(RegisterSetClasses[*it2]);
> }
>
> @@ -1152,8 +1175,8 @@ buildRegisterClasses(SmallPtrSet<Record*
> Record *Def = RC.getDef();
> if (!Def)
> continue;
> - ClassInfo *CI = RegisterSetClasses[std::set<Record*>(RC.getOrder().begin(),
> - RC.getOrder().end())];
> + ClassInfo *CI = RegisterSetClasses[RegisterSet(RC.getOrder().begin(),
> + RC.getOrder().end())];
> if (CI->ValueName.empty()) {
> CI->ClassName = RC.getName();
> CI->Name = "MCK_" + RC.getName();
> @@ -1165,7 +1188,7 @@ buildRegisterClasses(SmallPtrSet<Record*
> }
>
> // Populate the map for individual registers.
> - for (std::map<Record*, std::set<Record*> >::iterator it = RegisterMap.begin(),
> + for (std::map<Record*, RegisterSet>::iterator it = RegisterMap.begin(),
> ie = RegisterMap.end(); it != ie; ++it)
> RegisterClasses[it->first] = RegisterSetClasses[it->second];
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list