[PATCH] Refactor bitcode reader to simplify control.

[Karl Schimpf]

================
Comment at: include/llvm/Support/StreamingMemoryObject.h:75
@@ +74,3 @@
+      size_t NextChunkSize = kChunkSize;
+      if (ObjectSize && ObjectSize < BytesRead + kChunkSize) {
+        NextChunkSize = ObjectSize - BytesRead;
----------------
rafael wrote:
> Why do you need this? The streamer will return how many bytes were read and can handle a larger request.
> 
> Also, why does it need to be part of this patch? It looks like this patch has many independent changes in it.
This was added to handle the case of when one is parsing a wrapped bitcode file. In such cases, you do not need to do another read (which may block until it succeeds). That was the intent of this change. However, a simpler approach would be to allow the extra read, and then not set ObjectSize (below) if already set.

I will remove this change, and add the conditional assignment to ObjectSize below.

I changed it in this CL because it didn't cause a problem until I fixed that materializing a module (when streaming) didn't actually read all of the bitcode file. When that change was added, tests failed and this issue was exposed.

I will remove the changes StreamingMemoryObject.{h,cpp} and put in a separate CL.

================
Comment at: include/llvm/Support/StreamingMemoryObject.h:88
@@ -79,3 +87,3 @@
     }
-    return Pos < BytesRead;
+    return (Pos < BytesRead) || (ObjectSize && Pos < ObjectSize);
   }
----------------
rafael wrote:
> Why the extra logic? If objectsize is known it is the same as BytesRead, no?
No, they aren't necessarily the same.

The problem happens when you have a wrapped bitcode file, and was not exposed until I fixed the case that we weren't reading the entire bitcode when materializing lazily. Then, a bunch of test cases failed.

When I looked into it, this is what I discovered:

1) The wrapped bitcode was smaller than kChunkSize. Hence, the initial read set BytesRead to the size of the wrapped file on first read.

2) The wrapper was then read, and set ObjectSize, which corresponded to 4 bytes smaller than BytesRead.

This is the reason I changed this file as I did.

http://reviews.llvm.org/D8786

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/