[PATCH] D28083: Add an index for Module Metadata record in the bitcode

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 28 17:51:38 PST 2016


> On Dec 28, 2016, at 5:22 PM, Teresa Johnson <tejohnson at google.com> wrote:
> 
> 
> 
> On Wed, Dec 28, 2016 at 5:07 PM, Mehdi AMINI via Phabricator <reviews at reviews.llvm.org <mailto:reviews at reviews.llvm.org>> wrote:
> mehdi_amini added a comment.
> 
> FYI: had trouble with:
> 
> 1. Blocks that contains no record (or very little): the placeholder may not be flushed yet and the back patch will fail. I added a threshold optimization which workaround this (r290690)
>  
> ExitBlock always calls FlushToWord. So maybe this is a good reason to put the metadata index in its own block. As a side effect, the offset then can be in number of 32-bit words (as for the VST offset), and you only need a 32-bit index offset.
>  
> 2. On 32 bits platform, we can't have fixed fields of 64 bits, I reverted the change in EmitAbbreviatedField from this patch, and instead emit now 2 fields of 32 bits each (r290693)
> 
> I think Emit64 should just be removed then...it didn't have any callers before your original patch, and this may be why. 

This seems safe though, it is emitting the 64bits value as two 32 bits one.
I don’t think there is good reason for the 32 bits limits but it is just not handled by all the pieces of the implementation (in my case it was breaking in the reader).

I removed Emit64 in r290701.

— 
Mehdi

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161228/6865fa0f/attachment.html>


More information about the llvm-commits mailing list