llvm dev - Oct 2012 - [LLVMdev] [PATCH / PROPOSAL] bitcode encoding that is ~15% smaller for large bitcode files...

On Sun, Sep 30, 2012 at 9:09 AM, Chris Lattner <clattner at apple.com>
wrote:
>
> On Sep 25, 2012, at 5:08 PM, Jan Voung <jvoung at chromium.org>
wrote:
>
> Hi all,
>
> I've been looking into how to make llvm bitcode files smaller.  There
is
> one simple change that appears to shrink linked bitcode files by about 15%.
>  See this spreadsheet for some rough data:
>
>
>
https://docs.google.com/spreadsheet/ccc?key=0AjRrJHQc4_bddEtJdjdIek5fMDdIdFFIZldZXzdWa0E
>
>
> The change is in how operand ids are encoded in bitcode files.  Rather
> than use an "absolute number" given by the ValueEnumerator, which
counts up
> within a function, we can have the id be relative to the current
> instruction.
>
>
> Very cool.  This is the "obviously right" way to do it, I'm
glad you've
> implemented it.  It looks like Duncan gave you a lot of feedback already,
> I'm looking forward to seeing the updated patch.  Thanks for tackling
this
> Jan!
>
> -Chris
>

Thanks for taking a look!  I wasn't sure of better way to toggle this
behavior at the module level vs the function level.  The options I toyed
with were:

  (a) bump MODULE_CODE_VERSION from 0 to 1
  (b) add a new optional module code to identify the change
  (c) stick with the approach in the first patch, which used a new function
block ID value, then check that all the function blocks uniformly have the
newer ID instead of mix of the old and new.


for option (a) there hasn't been a version bump before(?) and I wasn't
sure
when that is appropriate.
for option (b) it seems like it's really indicating a version change and
not doing anything else, so could have been done more directly with (a)
for option (c) it ends up creating a new block ID that overlaps with the
existing function one and that would be first one to do that?

Thanks again Duncan for the review, and Renato for the suggestion of
looking into SLEB. Attached is an updated patch which toggles the change at
the module level (done with option a), and uses signed VRBs for phis,

- Jan
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121001/72a138aa/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: relative_ids.patch4
Type: application/octet-stream
Size: 36425 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121001/72a138aa/attachment.obj>

> Thanks for taking a look!  I wasn't sure of better way to toggle this
> behavior at the module level vs the function level.  The options I toyed
> with were:
>
>   (a) bump MODULE_CODE_VERSION from 0 to 1
>   (b) add a new optional module code to identify the change
>   (c) stick with the approach in the first patch, which used a new function
> block ID value, then check that all the function blocks uniformly have the
> newer ID instead of mix of the old and new.
>
>
> for option (a) there hasn't been a version bump before(?) and I
wasn't sure
> when that is appropriate.
> for option (b) it seems like it's really indicating a version change
and not
> doing anything else, so could have been done more directly with (a)
> for option (c) it ends up creating a new block ID that overlaps with the
> existing function one and that would be first one to do that?
>
> Thanks again Duncan for the review, and Renato for the suggestion of
looking
> into SLEB. Attached is an updated patch which toggles the change at the
> module level (done with option a), and uses signed VRBs for phis,
This is awesome! I tried it by creating a .ll of all of clang. Some sizes:

* clang.ll        490M
* clang.ll.gz   55M
* clang.ll.bz2 37M
* clang.ll.xz   27M

with the current bitcode:

* clang.bc      82M
* clang.bc.xz 27M

with the new bitcode:

* clang.bc 70M
* clang.bc.xz 26M

it looks like a nop run of opt also got a bit faster, from 10.451s to 10.176s.

+static void EmitSignedInt64(SmallVectorImpl<uint64_t> &Vals, uint64_t
V) {

Please start function names with a lower case letter.

+EnableOldStyleFunctions("enable-old-style-functions",

Maybe something like use-abs-operands is a better name now that this
is a global setting, no?
> - Jan
Cheers,
Rafael

On Sat, Oct 6, 2012 at 8:32 AM, Rafael Espíndola <rafael.espindola at
gmail.com
> wrote:
>
> +static void EmitSignedInt64(SmallVectorImpl<uint64_t> &Vals,
uint64_t V) {
>
> Please start function names with a lower case letter.
>
>
Done -- changed this function and most of the "pushValue" functions. 
I
left PushValueAndType alone since that is an existing function and it
seemed like that would introduce other unrelated changes.

> +EnableOldStyleFunctions("enable-old-style-functions",
>
> Maybe something like use-abs-operands is a better name now that this
> is a global setting, no?
>
Yes that would be more descriptive.  Changed in new patch.

>
> > - Jan
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121008/0599d7ba/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: relative_ids.patch5
Type: application/octet-stream
Size: 38577 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121008/0599d7ba/attachment.obj>