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

Thanks for the review Rafael!

Chris, did you want to take a look at the patch too?

Thanks,
- Jan


On Wed, Oct 10, 2012 at 12:39 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:
> On 10 October 2012 15:15, Jan Voung <jvoung at chromium.org> wrote:
> > Yes, I had about 133K hits for INST_PHI with a negative value, out of
> 136K
> > hits of any "INST_.*" with a negative valued operand.
> >
> > Overall there were 474K INST_PHI and 12 million "INST_.*" in
my tests.
>
> Cool!
>
> Thanks again for working on this!
>
> > - Jan
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121010/7b9db3ec/attachment.html>

On Oct 10, 2012, at 5:10 PM, Jan Voung <jvoung at chromium.org> wrote:
> Thanks for the review Rafael!
> 
> Chris, did you want to take a look at the patch too?
Your most recent patch looks good to me, with a minor change:

+++ b/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -33,6 +33,13 @@
 using namespace llvm;
 
 static cl::opt<bool>
+UseAbsoluteOperandIDs("use-abs-operands",
+                      cl::desc("Assign instruction operand IDs with the
older "
+                               "absolute encoding instead of using "
+                               "relative IDs."),
+                      cl::init(false), cl::Hidden);

Please remove this, the new writer should just always generate the relative
encoding.  Please feel free to land the patch with this change.

Also, this is a significant enough thing that it would be good to add something
to the bitcode docs (in the IR section,
http://llvm.org/docs/BitCodeFormat.html#llvm-ir-encoding) that describes the old
and new format.

Thanks again for tackling this!

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121010/303678d1/attachment.html>

On Wed, Oct 10, 2012 at 9:59 PM, Chris Lattner <clattner at apple.com>
wrote:
>
> On Oct 10, 2012, at 5:10 PM, Jan Voung <jvoung at chromium.org>
wrote:
>
> Thanks for the review Rafael!
>
> Chris, did you want to take a look at the patch too?
>
>
> Your most recent patch looks good to me, with a minor change:
>
> +++ b/lib/Bitcode/Writer/BitcodeWriter.cpp
> @@ -33,6 +33,13 @@
>  using namespace llvm;
>
>
>  static cl::opt<bool>
> +UseAbsoluteOperandIDs("use-abs-operands",
> +                      cl::desc("Assign instruction operand IDs with
the
> older "
> +                               "absolute encoding instead of using
"
> +                               "relative IDs."),
> +                      cl::init(false), cl::Hidden);
>
> Please remove this, the new writer should just always generate the
> relative encoding.  Please feel free to land the patch with this change.
>
>
Also, this is a significant enough thing that it would be good to
add
> something to the bitcode docs (in the IR section,
> http://llvm.org/docs/BitCodeFormat.html#llvm-ir-encoding) that describes
> the old and new format.
>
> Thanks again for tackling this!
>
> -Chris
>
Thanks!  Removed the flag and committed as 165739.  Will update the docs.

- Jan
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121011/0d0202c1/attachment.html>