[PATCH] D65994: Extended FPOptions with new attributes

Wed Mar 11 08:41:01 PDT 2020

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

>    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)
>
>
> Rounding (5 standard variants) and exception (3 variants) already do not
> fit 4 bits.
>
Sure they do: there are 5 x 3 = 15 combinations, which can be encoded in 4
bits. This would be a short-term fix only as it sounds like the number of
bits will grow further.

Fortunately Melanie is working on a better/more scalable solution of
putting the FPOptions in trailing objects of the nodes where they're
actually used (e.g. most BinaryOperators aren't floating point at all).
Then FPOptions can grow as many bits as needed without affecting many nodes.

> And there is also precision, denormals treatment, exception mask, they are
> usually represented in hardware register and
> make up a floating point environment. Instead of putting all these bits to
> different places it would be better to collect them in one place, it could
> make operations simpler and faster.
>
>   both rounding and exceptions seem essentially unused at present.
>
>
> Implementation of advanced floating point support is in progress, they
> will be used more and more.
>
That sounds great! Sorry, I didn't mean to imply at all they were useless,
just as an uninformed person I can't reason about them by looking at uses
as they don't exist yet.

  (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*)
>
>
> As objects are allocated aligned, if size in not multiple of the
> alignment, some part of memory is spent uselessly anyway.
>
Right. My point is that today, BinaryOperator uses exactly all its bits.
Tomorrow, we need to add a HasError bit to Expr. So tomorrow, the marginal
cost of the last bit (or savings from squeezing the last bit) is 64 bits,
at least for BinOp. This is certainly a short-term view, the long-term
option is probably moving FPOptions out of the bitfields in some way.

Use cases vary, and many tools deal with large ASTs but don't use LLVM
>> codegen at all.
>
>
> That is true, but developer tools are usually less restricted in computing
> resources. Besides hard economy in memory often means some loss of
> performance.
>
I'm not sure this is productive - if you don't think bit-packing is a
sensible tradeoff, then I think to some extent it's on you to change it,
rather than eat all the bits and walk away! I do happen to work on
developer tools with memory constraints, but that's not the hat I'm wearing
today - we're just stuck between the established design (which aims for
compactness) and existing uneconomical use of it.

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.
>>
>>
> We could keep pointer to shared properties, thus saving memory. Moreover
> such facility could enable some techniques using this shared property as a
> marker. For example, all floating point operations obtained from the same
> region where `pragma STDC FENV_ACCESS` is in act could point to the same
> FPOption object, thus we could distinguish between different regions. It
> would save memory consumption without sacrifice of maintainability and
> speed.
>
Yeah, just concretely needing an 8-byte pointer to a 1-byte shared
structure doesn't save memory, so some larger refactoring/design would be
needed. TrailingObjects is simpler.

> Thanks,
> --Serge
>
>
> On Wed, Mar 11, 2020 at 8:01 PM Sam McCall <sammccall at google.com> wrote:
>
>> 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/ce4c3cb5/attachment-0001.html>