[PATCH] Use NativeReferenceIvarsV2 if necessary.

[Shankar Kalpathi Easwaran]

  Just saving this in the review thread.

  >
  >
  > ================

  Comment at: lib/ReaderWriter/Native/WriterNative.cpp:175-185
  > @@ +174,13 @@
  > +  // possible.
  > +  void maybeConvertReferencesToV1() {
  > +    std::set<int64_t> addends;
  > +    for (const NativeReferenceIvarsV2 &ref : _referencesV2) {
  > +      if (!canConvertReferenceToV1(ref))
  > +        return;
  > +      addends.insert(ref.addend);
  > +      if (addends.size() >= UINT16_MAX)
  > +        return;
  > +    }
  > +    convertReferencesToV1();
  > +  }
  > +
  > ----------------
  > This converts either all references to V1 or everything remains V2. We
  > might want to convert a part of the references if the targetIndex <
  > UINT16_MAX and leave the rest to V2, if the targetIndex is higher than 16
  > bits.
  >

  It feels that it's overly complicated to emit a mixed file of V1 and V2.
  It's going to be slower too because we need to look at the index of a
  reference to decide the reference is in V1 or V2.

  Let's calculate the size increase when V2 is used instead of V1. sizeof(V1)
  is 8. sizeof(V2) is 24. So, by converting 65535 V1 references to V2, the
  file grows 1MB. Actual increase would be smaller than that because V2 would
  get rid of the AddendTable. A file containing so much references is large
  anyway, I don't think less than 1MB increase would matter a lot for such
  file. Thus I'd think simplicity wins here.

  The native format is not really being used by anyone, so we don't know what
  is going to be an issue and what's not going to be. Implementing something
  to "optimize" Native format in such a way to increase the complexity of
  code would be a premature optimization at this moment, I think.



  _______________________________________________
  llvm-commits mailing list
  llvm-commits at cs.uiuc.edu
  http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

http://llvm-reviews.chandlerc.com/D2217