[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein abandoned this revision. hokein added a comment. woo, looks like the `IsOMPStructuredBlock` bit in `Stmt` was removed in https://reviews.llvm.org/rGd5edcb90643104d6911da5c0ff44c4f33fff992f, we can use the bit for `error`, so this patch is not needed anymore. Repository: rG LLVM Github

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D75443#1917351 , @mibintc wrote: > I ran into an obstacle trying to add Trailing storage onto binary operator. > it will probably take me longer. sure, no worries, and thanks for your help. It is not super urgent for us at th

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added a comment. I ran into an obstacle trying to add Trailing storage onto binary operator. it will probably take me longer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75443/new/ https://reviews.llvm.org/D75443 _

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments. Comment at: clang/include/clang/Basic/LangOptions.h:461 + constexpr static unsigned MaxExceptionValue = 3; + unsigned rounding_and_exceptions: 4; }; There are 5 rounding directions defined by IEEE-754, which we should eventuall

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D75443#1916909 , @mibintc wrote: > well, it would probably be quickest to separate the trailing storage into a > separate patch. i'll try to get that submitted today. shall i submit that > as a phabricator review or is there

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D75443#1916909 , @mibintc wrote: > well, it would probably be quickest to separate the trailing storage into a > separate patch. i'll try to get that submitted today. shall i submit that > as a phabricator review or is the

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a subscriber: sepavloff. sammccall added a comment. this looks reasonable to me, would like to give @mibintc and also @sepavloff a chance to chime in. Comment at: clang/include/clang/Basic/LangOptions.h:437 + LangOptions::FPExc

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added a comment. well, it would probably be quickest to separate the trailing storage into a separate patch. i'll try to get that submitted today. shall i submit that as a phabricator review or is there a different, preferred way to do that? Repository: rG LLVM Github Monorepo CHA

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D75443#1916829 , @sammccall wrote: > In D75443#1916761 , @mibintc wrote: > > > I'm working on a patch that will move FPOptions to Trailing storage, part > > of https://reviews.llvm.org/D7

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 249613. hokein marked 4 inline comments as done. hokein added a comment. address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75443/new/ https://reviews.llvm.org/D75443 Files: clang/include/clang/A

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D75443#1916761 , @mibintc wrote: > I'm working on a patch that will move FPOptions to Trailing storage, part of > https://reviews.llvm.org/D72841 ; hope to have another revision uploaded > today or tomorrow. I got feedback

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 249606. hokein added a comment. reduce FPOption from 8 bits to 7 bits, based on the offline discussion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75443/new/ https://reviews.llvm.org/D75443 Files: clang/in

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 249607. hokein added a comment. update a missing CXXOperatorCallExprBitfields, and fix typos. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75443/new/ https://reviews.llvm.org/D75443 Files: clang/include/clan

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. As discussed offline... this undoes a desirable optimization from D54526 . A couple of possible ways to avoid this: - FPOptions integer encoding can fit into 7 bits: combine integer behavior + exception behavior into 3 x 5 = 15 states

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-11 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added a comment. I'm working on a patch that will move FPOptions to Trailing storage, part of https://reviews.llvm.org/D72841 ; hope to have another revision uploaded today or tomorrow. I got feedback that moving the field to BinaryOperator isn't adequate. Repository: rG LLVM Githu

[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

2020-03-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. hokein edited the summary of this revision. The stmt bit-fields is full (max 64 bits) for BinaryOperator now, adding a new bit field (error) causes an 'static_assert(sizeof(*this) <=8)' violation i