[PATCH] Switch a std::map to a DenseMap in CodeGenRegisters

[Duncan P. N. Exon Smith]

> On 2015-Feb-13, at 00:08, Owen Anderson <resistor at mac.com> wrote:
> 
> 
>> On Feb 10, 2015, at 3:07 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> By inspection, it looks like the iteration order could eventually
>> affect the contents of `SubRegs`, depending on whether there could
>> be duplicates of `I->second` in `Comps`.  But I don't actually know
>> this code... maybe `I->second` is guaranteed to be unique?
> 
> What’s modeled here is the composition relationship of subregister indices.  Index A’s map contains the pair <B, C> to indicate that A o B —> C.  With that in mind, I can’t imagine a scenario in which it would be meaningful for C not to be unique.  If you think of it as subsetting operations, you need a subsetting operation A such that there were multiple sub-subsetting operations (B) that you could apply to end up at the net subsetting operation C (empty sets excluded).  As far as I can tell, that doesn’t make semantic sense.

I agree.  Might deserve a comment and/or assertions, such as:

    #ifndef NDEBUG
      DenseSet<CodeGenSubRegIndex *> UniquedSubsetResults;
    #endif
      for (const auto &I : Comps) {
        assert(UniquedSubsetResults.insert(I.second).second &&
               "Expected subset results to be unique");
        // Use I...
      }

to make the invariant obvious when reading the code.

>> The results of the first loop are obviously independent of iteration
>> order.  I'm not sure of the second... if `Map` has two entries with
>> the same `EnumValue` (but different `I->second`) the result would be
>> non-deterministic.
> 
> EnumValue has to be unique, as it is ultimately how the sub register indices are represented in the generated C++ code at the end of the day.

Makes sense to me.  Might be worth an assertion in the second loop
to make the invariant obvious:

    // All entries are compatible. Make it so.
    for (const auto &I : Map) {
      auto *&Entry = Vec[I.first->EnumValue - 1];
      assert((!Entry || Entry == I.second) && "Expected EnumValue to be unique");
      Entry = I.second;
    }

Otherwise this LGTM.