[PATCH] D65994: Extended FPOptions with new attributes

[Sam McCall via cfe-commits]

On Wed, Mar 11, 2020 at 12:42 PM Serge Pavlov <sepavloff at gmail.com> wrote:

> It is a matter of taste, but for me it looks strange to develop complex
> and error-prone system to save 7 bits at expense of readability and
> maintainability. My observations show that clang AST consumes much less
> memory than llvm objects.
>
I agree, and would be happy if we had a design without these scarce bits,
but AFAICS we don't. In this environment, 5 bits are quite a lot for FP
flexibility, and I think the complexity of reducing this is small and
isolated (rounding + exceptions together fit in 4 bits). But I ask about
the domain because maybe there's a simple but smaller model I can't really
infer this myself, both rounding and exceptions seem essentially unused at
present.
(Note that the cost here is not 7 bits per node but 63 - once the bits
available in Stmt are full the stmt subclass needs to gain a member, and
its alignment is 8 *bytes*)

Use cases vary, and many tools deal with large ASTs but don't use LLVM
codegen at all.

FPOption could be shared between user using something like shared_ptr, but
> it means expenses of 64 bit for a pointer. Don't know if it makes sense...
>
As you say, I don't think this saves anything, unless we can stop storing
the pointer in BinaryOperator.

Cheers, Sam


>
>
> Thanks,
> --Serge
>
>
> On Wed, Mar 11, 2020 at 6:30 PM Sam McCall via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> sammccall added a comment.
>>
>> This patch increased the used size of BinaryOperator by 5 bits.
>> Those bits were all padding, but now BinaryOperatorBitfields is
>> completely full at 32 bits and we can't add any new bits to Stmt without
>> increasing BinaryOperator by 8 bytes. (See D75443 <
>> https://reviews.llvm.org/D75443> and D54526 <
>> https://reviews.llvm.org/D54526> for the optimization this would revert).
>>
>> To squeeze in the new bit I'm planning to suggest squeezing getInt() to 7
>> bits (it encodes 3x2x5x3 = 90 states, so this is possible) but I'm not
>> really familiar with this domain - if many of the 90 states are not
>> possible it'd probably be useful to have some more bits back :-)
>>
>>
>> Repository:
>>   rG LLVM Github Monorepo
>>
>> CHANGES SINCE LAST ACTION
>>   https://reviews.llvm.org/D65994/new/
>>
>> https://reviews.llvm.org/D65994
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200311/4bf26926/attachment.html>