[PATCH] Use NativeReferenceIvarsV2 if necessary.

Rui Ueyama ruiu at google.com
Tue Nov 19 11:15:56 PST 2013


On Tue, Nov 19, 2013 at 9:02 AM, Shankar Kalpathi Easwaran <
shankarke at gmail.com> wrote:

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

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131119/5f6daf8b/attachment.html>


More information about the llvm-commits mailing list