Page MenuHomePhabricator

mibintc (Melanie Blower)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 14 2016, 12:37 PM (176 w, 1 d)

Recent Activity

Today

mibintc updated the diff for D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.

Here's another revision, I believe this handles all of @rjmccall 's requests. Thank you, John, for your review!

Tue, Mar 31, 1:44 PM · Restricted Project

Sat, Mar 28

mibintc added a comment to D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.

On my RHEL8 system building Debug clang;clang-tools-extra, check-all is passing.

Sat, Mar 28, 8:04 AM · Restricted Project
mibintc added a comment to D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.

I added some inline replies to John. I'm not certain I have everything yet exactly the way he wants it.

Sat, Mar 28, 7:00 AM · Restricted Project
mibintc updated the diff for D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.

@rjmccall Many thanks for your code review. Please take a look at this when you get a chance.

Sat, Mar 28, 7:00 AM · Restricted Project

Wed, Mar 25

mibintc added inline comments to D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.
Wed, Mar 25, 9:43 AM · Restricted Project
mibintc added a comment to D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.

I don't see those Harbormaster fails, using trunk and RHEL8 on architecture x86_64, Debug build of clang;clang-tools-extra and check-all. I ran all the failed lit tests separately and each passed

Wed, Mar 25, 9:10 AM · Restricted Project
mibintc added a comment to D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.
Wed, Mar 25, 7:32 AM · Restricted Project

Tue, Mar 24

mibintc updated the diff for D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.

I think this addresses all the feedback from @rjmccall

Tue, Mar 24, 6:47 PM · Restricted Project

Mon, Mar 23

mibintc added a comment to D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.

Let's make this patch be purely a representation change. I wouldn't expect *any* test changes from that; if there are, we should understand why. You can then add the stuff about tracking whether there's a pragma in a separate patch.

Mon, Mar 23, 7:03 AM · Restricted Project

Fri, Mar 20

mibintc updated the diff for D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.

This revision fixes the clang-tidy,clang-format and removes the redundant accessors. Ready for your review.

Fri, Mar 20, 1:34 PM · Restricted Project
mibintc added a comment to D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.

There's a bug in this patch that i haven't solved yet. I think it's just 1 bug. The bug is that the AST statement visitor is going to the "fallback" instead of the right destination when walking CompoundAssignmentOperator. This patch collapses CompoundAssignmentOperator class into BinaryOperator class. In a spot check, i think all the failing tests are because the Visitor walk isn't working correctly for CompoundAssign (for example += ). These are the test fails reported by check-clang in my sandbox,
Failing Tests (15):

Clang :: Analysis/loopexit-cfg-output.cpp
Clang :: CXX/drs/dr2xx.cpp
Clang :: CXX/expr/expr.const/p2-0x.cpp
Clang :: CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p17.cpp
Clang :: CodeGen/ffp-contract-fast-option.cpp
Clang :: CodeGenCXX/const-init-cxx1y.cpp
Clang :: CodeGenCXX/const-init-cxx2a.cpp
Clang :: CodeGenCXX/non-const-init-cxx2a.cpp
Clang :: Sema/warn-unreachable.c
Clang :: Sema/warn-unsequenced.c
Clang :: SemaCXX/constant-expression-cxx1y.cpp
Clang :: SemaCXX/constant-expression-cxx2a.cpp
Clang :: SemaCXX/decomposed-condition.cpp
Clang :: SemaCXX/integer-overflow.cpp
Clang :: SemaCXX/warn-unsequenced.cpp
Fri, Mar 20, 10:50 AM · Restricted Project
mibintc updated the diff for D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.

This revision has been rebased, the LIT test failures seen in the previous revision have been corrected: check-all passes. Still owe you some polishing. I'm going to add a couple inline comments.

Fri, Mar 20, 10:49 AM · Restricted Project

Wed, Mar 18

mibintc created D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.
Wed, Mar 18, 1:35 PM · Restricted Project
mibintc added a comment to D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage.

There's a bug in this patch that i haven't solved yet. I think it's just 1 bug. The bug is that the AST statement visitor is going to the "fallback" instead of the right destination when walking CompoundAssignmentOperator. This patch collapses CompoundAssignmentOperator class into BinaryOperator class. In a spot check, i think all the failing tests are because the Visitor walk isn't working correctly for CompoundAssign (for example += ). These are the test fails reported by check-clang in my sandbox,
Failing Tests (15):

Clang :: Analysis/loopexit-cfg-output.cpp
Clang :: CXX/drs/dr2xx.cpp
Clang :: CXX/expr/expr.const/p2-0x.cpp
Clang :: CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p17.cpp
Clang :: CodeGen/ffp-contract-fast-option.cpp
Clang :: CodeGenCXX/const-init-cxx1y.cpp
Clang :: CodeGenCXX/const-init-cxx2a.cpp
Clang :: CodeGenCXX/non-const-init-cxx2a.cpp
Clang :: Sema/warn-unreachable.c
Clang :: Sema/warn-unsequenced.c
Clang :: SemaCXX/constant-expression-cxx1y.cpp
Clang :: SemaCXX/constant-expression-cxx2a.cpp
Clang :: SemaCXX/decomposed-condition.cpp
Clang :: SemaCXX/integer-overflow.cpp
Clang :: SemaCXX/warn-unsequenced.cpp
Wed, Mar 18, 1:35 PM · Restricted Project

Thu, Mar 12

mibintc added a comment to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

@David.Bolvansky told me “https://lnt.llvm.org/ -> Test suite nts -> watch for

LNT-Broadwell-AVX2-O3clang_PRODx86_64:1364”

Thu, Mar 12, 1:01 PM · Restricted Project
mibintc added a comment to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

Revisiting this patch. I think that before fixing the -ffp-contract=off problem I originally raised here, there are two questions that have come up in this discussion that we should first resolve. Specifically:

(1) Do we want to enable FMA transformations by default (at appropriate optimization levels), like GCC does? That is, should FMA be done for targets that support it with a command as simple as the following?

clang -O2 -c test.c

This has been discussed/tried a few times including very recently. I'm not sure where we stand currently, but here's some background:
https://reviews.llvm.org/rL282259
D74436 - cc @mibintc

And (sorry for the various flavors of links to the dev-lists, but that's how it showed up in the search results):
http://lists.llvm.org/pipermail/llvm-dev/2017-March/111129.html
http://lists.llvm.org/pipermail/cfe-dev/2020-February/064645.html
https://groups.google.com/forum/#!msg/llvm-dev/nBiCD5KvYW0/yfjrVzR7DAAJ
https://groups.google.com/forum/#!topic/llvm-dev/Aev2jQYepKQ
http://clang-developers.42468.n3.nabble.com/RFC-FP-contract-on-td4056277.html

Thu, Mar 12, 10:51 AM · Restricted Project
mibintc added a comment to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

This patch, which hasn't been committed, contains modifications to the UserManual with many details concerning 'floating point semantic modes" and the relation to floating point command line options. This is from a discussion that @andrew.w.kaylor initiated on the discussion lists. https://reviews.llvm.org/D74436

Thu, Mar 12, 10:51 AM · Restricted Project
mibintc updated subscribers of D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.
Thu, Mar 12, 10:51 AM · Restricted Project

Wed, Mar 11

mibintc added a comment to D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC..

I ran into an obstacle trying to add Trailing storage onto binary operator. it will probably take me longer.

Wed, Mar 11, 10:45 AM · Restricted Project
mibintc added a comment to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

@rjmccall Since CompoundAssignmentOperator derives from BinaryOperator, it's not simple to add Trailing storage here. I think I will have to fold CompoundAssignmentOperator into BinaryOperator and then add the 2 extra fields needed by CompoundAssignmentOperator into Trailing storage. Can you think of a better way? I worked on Trailing storage for UnaryOperator first and that wasn't too bad, but Binary is a different story.

Wed, Mar 11, 10:45 AM · Restricted Project, Restricted Project
mibintc added a comment to D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC..

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?

Wed, Mar 11, 7:58 AM · Restricted Project
mibintc added a comment to D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC..

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.

Wed, Mar 11, 6:17 AM · Restricted Project

Mon, Mar 9

mibintc added a comment to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

@rjmccall suggested that I needed to remove FPOptions from the Stmt class since the sizeof assertion failed. I moved that information into the relevant expression nodes and fixed a few small things that I didn't like in the previous version.

You only need to do this for the expression nodes where it causes an overflow in the size of the bit-field; I don't think you're overflowing the capacity of UnaryOperatorBitfields, for example.

Mon, Mar 9, 7:30 AM · Restricted Project, Restricted Project

Thu, Mar 5

mibintc added a comment to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

I got an email like this "Harbormaster failed remote builds in B48237: Diff 248536!" but there is no further information. It builds OK from my workstation. I did have to paste the review because an upload to Phabricator exceeded the size limit, could that be why? Is there a way to get more information about the build failure?

Thu, Mar 5, 12:39 PM · Restricted Project, Restricted Project
mibintc added inline comments to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.
Thu, Mar 5, 10:57 AM · Restricted Project, Restricted Project
mibintc updated the diff for D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

@rjmccall suggested that I needed to remove FPOptions from the Stmt class since the sizeof assertion failed. I moved that information into the relevant expression nodes and fixed a few small things that I didn't like in the previous version.

Thu, Mar 5, 10:22 AM · Restricted Project, Restricted Project

Wed, Mar 4

mibintc added a comment to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

some inline replies and comments

Wed, Mar 4, 1:33 PM · Restricted Project, Restricted Project
mibintc updated the diff for D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

Responding to @rjmccall 's review. Previously there was a FIXME about adding FPFeatures to UnaryOperator. This seems like the right time to do that, so that change is included there. I'll add inline replies too.

Wed, Mar 4, 1:29 PM · Restricted Project, Restricted Project

Feb 27 2020

mibintc added a comment to D73967: Implement _ExtInt as an extended int type specifier..

Added a couple inline comments

Feb 27 2020, 7:05 AM

Feb 24 2020

mibintc updated the diff for D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

This patch is code complete and ready for your review and consideration.

Feb 24 2020, 1:06 PM · Restricted Project, Restricted Project
mibintc committed rGc8dadac228b7: add release notes for ffp-model and ffp-exception-behavior (authored by mibintc).
add release notes for ffp-model and ffp-exception-behavior
Feb 24 2020, 6:50 AM

Feb 18 2020

mibintc committed rGcd2c5af6dfd6: Reland D74436 "Change clang option -ffp-model=precise to select ffp… (authored by mibintc).
Reland D74436 "Change clang option -ffp-model=precise to select ffp…
Feb 18 2020, 7:02 AM
mibintc added a reviewer for D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level: arsenm.
Feb 18 2020, 7:02 AM · Restricted Project, Restricted Project

Feb 16 2020

mibintc updated the diff for D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

I found the problem in the #pragma float_control (push/pop) stack, it was just a dumb bug.

Feb 16 2020, 12:05 PM · Restricted Project, Restricted Project

Feb 14 2020

mibintc committed rG9122b92f8e04: Revert "Reland D74436 "Change clang option -ffp-model=precise to select ffp… (authored by mibintc).
Revert "Reland D74436 "Change clang option -ffp-model=precise to select ffp…
Feb 14 2020, 7:37 AM
mibintc added a reverting change for rG0a1123eb43f9: Reland D74436 "Change clang option -ffp-model=precise to select ffp-contract=on": rG9122b92f8e04: Revert "Reland D74436 "Change clang option -ffp-model=precise to select ffp….
Feb 14 2020, 7:37 AM

Feb 13 2020

mibintc added a comment to D74436: Change clang option -ffp-model=precise to select ffp-contract=on.

@jsji Will your backend tolerate -ffp-contract=on if optimizations are not disabled, e.g. -O2? We are proposing that the default floating point model be -ffp-model=precise, and with precise model, the ffp-contract=on. However you are right we don't want the frontend to create FMA when optimizations are disabled -O0

Feb 13 2020, 3:18 PM · Restricted Project
mibintc committed rG88ec01ca1bfa: Revert "Revert "Revert "Change clang option -ffp-model=precise to select ffp… (authored by mibintc).
Revert "Revert "Revert "Change clang option -ffp-model=precise to select ffp…
Feb 13 2020, 3:10 PM
mibintc added a reverting change for rGabd09053bc7a: Revert "Revert "Change clang option -ffp-model=precise to select ffp…: rG88ec01ca1bfa: Revert "Revert "Revert "Change clang option -ffp-model=precise to select ffp….
Feb 13 2020, 3:10 PM
mibintc added a comment to D74436: Change clang option -ffp-model=precise to select ffp-contract=on.

This is also breaking test-suites in our internal buildbots ..

This change is actually changing the default behavior: if user does NOT supply -ffp-contract, it becomes -ffp-contract=on.

So this means any code without explicitly add -ffp-contract=off will have -ffp-contract=on.
eg: -O0 will also generate FMA.

Is this intended behavior change? @mibintc @andrew.w.kaylor

Feb 13 2020, 3:00 PM · Restricted Project

Feb 12 2020

mibintc committed rGa0d913a1ace9: Fix regression due to reviews.llvm.org/D74436 by adding option ffp-contract=off… (authored by mibintc).
Fix regression due to reviews.llvm.org/D74436 by adding option ffp-contract=off…
Feb 12 2020, 7:14 PM
mibintc committed rGabd09053bc7a: Revert "Revert "Change clang option -ffp-model=precise to select ffp… (authored by mibintc).
Revert "Revert "Change clang option -ffp-model=precise to select ffp…
Feb 12 2020, 7:37 AM
mibintc added a reverting change for rG99c5bcbce89f: Revert "Change clang option -ffp-model=precise to select ffp-contract=on": rGabd09053bc7a: Revert "Revert "Change clang option -ffp-model=precise to select ffp….
Feb 12 2020, 7:37 AM
mibintc updated the diff for D74436: Change clang option -ffp-model=precise to select ffp-contract=on.
Feb 12 2020, 7:36 AM · Restricted Project
mibintc added a comment to D74436: Change clang option -ffp-model=precise to select ffp-contract=on.

some replies to Andy. I'll upload another patch here which passed check-all locally. then i'll re-commit it.

Feb 12 2020, 7:36 AM · Restricted Project

Feb 11 2020

mibintc committed rG99c5bcbce89f: Revert "Change clang option -ffp-model=precise to select ffp-contract=on" (authored by mibintc).
Revert "Change clang option -ffp-model=precise to select ffp-contract=on"
Feb 11 2020, 2:23 PM
mibintc added a reverting change for rG3fcdf2fa945a: Change clang option -ffp-model=precise to select ffp-contract=on: rG99c5bcbce89f: Revert "Change clang option -ffp-model=precise to select ffp-contract=on".
Feb 11 2020, 2:23 PM
mibintc committed rG3fcdf2fa945a: Change clang option -ffp-model=precise to select ffp-contract=on (authored by mibintc).
Change clang option -ffp-model=precise to select ffp-contract=on
Feb 11 2020, 2:13 PM
mibintc closed D74436: Change clang option -ffp-model=precise to select ffp-contract=on.
Feb 11 2020, 2:13 PM · Restricted Project
mibintc retitled D74436: Change clang option -ffp-model=precise to select ffp-contract=on from Change clang default to -ffp-model=precise to Change clang option -ffp-model=precise to select ffp-contract=on.
Feb 11 2020, 1:10 PM · Restricted Project
mibintc added a comment to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

I would think contract change can be separated from the rest of the changes, and therefore should be a separate review (to reduce noise)?

Feb 11 2020, 12:51 PM · Restricted Project, Restricted Project
mibintc created D74436: Change clang option -ffp-model=precise to select ffp-contract=on.
Feb 11 2020, 12:51 PM · Restricted Project
mibintc updated the diff for D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

This patch is a work in progress.

Feb 11 2020, 8:54 AM · Restricted Project, Restricted Project

Jan 28 2020

mibintc added a comment to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

It's not clear to me from reading this how the "precise" control is going to work with relation to the fast math flags. I don't think MSVC allows the granularity of control over these flags that we have in clang, so maybe they don't have this problem.

You're right, MSVC only allows the pragma at file scope.

Consider this code: https://godbolt.org/z/mHiLCm

With the options "-ffp-model=precise -fno-honor-infinities -fno-honor-nans" the math operations here are generated with the "nnan ninf contract" flags. That's correct. What will happen when I use the pragma to turn precise off? Does it enable all fast math flags? Will the subsequent "pop" leave the "ninf nnan" fast math flags enabled?

This patch doesn't have support for turning precise off, that's a bug, I will revisit. This is my plan for how to handle enabling/disabling fast math: IRBuilder.h has settings FMF, and also supplies clearFastMathFlags, setFastMathFlags(flags) and FastMathFlagGuard. When the expression is walked that alters the FMF, use FastMathFlagGuard to save the current state of FMF, modify the settings using the clear or set functions, walk the expression. After the expression is walked, the FMF settings will be restored to previous state by the FastMathFlagGuard destructor.

As I said, I don't think you can get into this situation with MSVC. I believe that icc will go into full fast math mode with the "precise, off, push" pragma but will go back to "nnan ninf contract" mode with the pop. At least, that's what the design docs say. I haven't looked at the code to verify this. It seems like the correct behavior in any case. I think the clang FPOptions needs individual entries for all of the fast math flags to handle this case.

Thanks I'll investigate this and add test cases. I think possibly since IRBuilder has the FMF like I described above it might work with current support. Is there currently a way to modify nan and inf at the source level or only by compiler option? BTW I've been asked to implement a pragma to control fp "reassoc" at the source level. I'm planning to do that after this patch is complete.

Jan 28 2020, 10:11 AM · Restricted Project, Restricted Project

Jan 27 2020

mibintc updated the diff for D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

rebase per @sepavloff request

Jan 27 2020, 10:39 AM · Restricted Project, Restricted Project

Jan 24 2020

mibintc added a comment to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

I don't see tests for correctness of the pragma stack (pragma float_control(... push), pragma float_control(pop)). Can you add them?

Jan 24 2020, 11:01 AM · Restricted Project, Restricted Project
mibintc updated the diff for D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.
Jan 24 2020, 10:43 AM · Restricted Project, Restricted Project

Jan 22 2020

mibintc added a comment to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

A couple inline replies to @sepavloff ; I'll be uploading another revision.

Jan 22 2020, 1:10 PM · Restricted Project, Restricted Project

Jan 21 2020

mibintc added inline comments to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.
Jan 21 2020, 12:51 PM · Restricted Project, Restricted Project
mibintc updated the diff for D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

Respond to review from @sepavloff

Jan 21 2020, 12:42 PM · Restricted Project, Restricted Project
mibintc added inline comments to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.
Jan 21 2020, 11:55 AM · Restricted Project, Restricted Project

Jan 20 2020

mibintc added a comment to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

Added inline reply

Jan 20 2020, 9:02 AM · Restricted Project, Restricted Project

Jan 17 2020

mibintc added a comment to D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.

@sepavloff Thanks a lot for your comments. I added a few replies and I have one question, added inline.

Jan 17 2020, 8:25 AM · Restricted Project, Restricted Project

Jan 16 2020

mibintc added a reviewer for D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level: anemet.
Jan 16 2020, 1:07 PM · Restricted Project, Restricted Project
mibintc created D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level.
Jan 16 2020, 6:42 AM · Restricted Project, Restricted Project

Dec 20 2019

mibintc added a comment to D69272: Enable '#pragma STDC FENV_ACCESS' in frontend.
In D69272#1717021, @kpn wrote:

Is there a way forward to support having the #pragma at the start of any block inside a function? The effect won't be restricted to that block, true, but the standard does say the #pragma is allowed.

Please clarify: I understand that the backend wants all floating point operations to be built using Floating Point Constrained Intrinsics if any operations use constrained intrinsic. But I thought that, if the constraint were only to apply to a block within the function, that the operations outside the block would be written with the default setting for rounding mode and exception behavior, and the operations inside the constrained block would be created with different settings. Like this
float f( float a, float b) {
a*b ; this operation is written with floating point constrained intrinsic, rounding mode nearest, exception behavior ignore
{
#pragma float_control ...
set exception behavior to strict
a*b; // this operation is written with floating point constrained intrinsic, rounding mode tonearest, exception behavior strict
}}

Dec 20 2019, 7:48 AM · Restricted Project

Dec 18 2019

mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

It seems the discussion of whether or not this is incomplete died out -- I'd prefer to assume it is incomplete if there is no consensus. Mailed D71635 to rename -frounding-math to -fexperimental-rounding-math.

Alternatively we could remove the warning. I still don't see a good argument for the middle ground of having it called -frounding-math but also generate a warning.

It's definitely incomplete but the results will not be any worse than you get when -frounding-math is ignored.

My preference would be to change the text of the warning that is issued but allow -frounding-math to be enabled by this commit without requiring an additional option.

If other reviewers agree, then let's just remove the warning. I can send a patch tomorrow unless someone else wants to do that.

Mailed D71671 to do this.

If neither patch is acceptable, then I would like to revert this commit instead, as we are having issues with this patch.

Dec 18 2019, 12:34 PM · Restricted Project, Restricted Project
mibintc accepted D71671: [clang] Remove -Wexperimental-float-control..
Dec 18 2019, 12:24 PM · Restricted Project
mibintc added a comment to D71671: [clang] Remove -Wexperimental-float-control..

thanks for taking care of this. looks good to me

Dec 18 2019, 12:24 PM · Restricted Project

Dec 12 2019

mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

I don't see a problem with this, but it would be nice to make the -f[no-]trapping-math command line option work. GNU compatibility is good.

Thanks Cameron, I'll go that way

Dec 12 2019, 5:19 AM · Restricted Project, Restricted Project

Dec 11 2019

mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

I've noticed you removed the change for CompilerInvocation.cpp about the initialization of the codegen option NoTrappingMath. Was that an accident?

Thanks again Michele. I'd like to get rid of Opts.NoTrappingMath, but I haven't been bold enough yet. NoTrappingMath is not expressive enough because it can hold only 2 values, whereas the Exception behavior can be ignore, strict or maytrap. So I'd get rid of that Opts field, and the only place where I see it actually being used is in llvm/lib/Target/ARM/ARMAsmPrinter.cpp and the change in this patch doesn't seem to affect the ARM logic so I think if I got rid of it, it would be OK. All the other instances of the string are in llvm test cases.

Dec 11 2019, 8:07 AM · Restricted Project, Restricted Project

Dec 9 2019

mibintc added a comment to D70451: [FPEnv] IRBuilder should not put strictfp on function definitions automatically.
In D70451#1775286, @kpn wrote:

@mibintc: This ticket is redundant now, correct? This was solved in the re-push of the command line option changes?

Dec 9 2019, 7:09 AM · Restricted Project
mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

I've noticed you removed the change for CompilerInvocation.cpp about the initialization of the codegen option NoTrappingMath. Was that an accident?

Dec 9 2019, 5:26 AM · Restricted Project, Restricted Project

Dec 5 2019

mibintc committed rG7f9b5138470d: Reapply af57dbf12e54 "Add support for options -frounding-math… (authored by mibintc).
Reapply af57dbf12e54 "Add support for options -frounding-math…
Dec 5 2019, 3:53 AM
mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

I fixed the lit test problem and pushed it again

Dec 5 2019, 3:52 AM · Restricted Project, Restricted Project

Dec 4 2019

mibintc added inline comments to D70451: [FPEnv] IRBuilder should not put strictfp on function definitions automatically.
Dec 4 2019, 12:54 PM · Restricted Project
mibintc committed rG5412913631fe: Revert " Reapply af57dbf12e54 "Add support for options -frounding-math… (authored by mibintc).
Revert " Reapply af57dbf12e54 "Add support for options -frounding-math…
Dec 4 2019, 12:27 PM
mibintc added a reverting change for rGcdbed2dd856c: Reapply af57dbf12e54 "Add support for options -frounding-math…: rG5412913631fe: Revert " Reapply af57dbf12e54 "Add support for options -frounding-math….
Dec 4 2019, 12:26 PM
mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

I reverted it again because build break on windows

Dec 4 2019, 12:25 PM · Restricted Project, Restricted Project
mibintc committed rGcdbed2dd856c: Reapply af57dbf12e54 "Add support for options -frounding-math… (authored by mibintc).
Reapply af57dbf12e54 "Add support for options -frounding-math…
Dec 4 2019, 11:39 AM
mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

I've pushed the updated patch,
commit cdbed2dd856c14687efd741c2d8321686102acb8

Dec 4 2019, 11:37 AM · Restricted Project, Restricted Project

Dec 2 2019

mibintc added a comment to D70451: [FPEnv] IRBuilder should not put strictfp on function definitions automatically.

I uploaded another version of https://reviews.llvm.org/D62731 which sets the strictfp attribute during clang/CodeGen so it's not needed during IRBuilder, and doesn't cause the problem with the instruction block parent not being set. The new version makes a change to IRBuilder.h

Dec 2 2019, 12:43 PM · Restricted Project
mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

I added inline comments describing what I did in this version of the patch to address the bug https://bugs.llvm.org/show_bug.cgi?id=44048

Dec 2 2019, 12:43 PM · Restricted Project, Restricted Project
mibintc added a reviewer for D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior: erichkeane.
Dec 2 2019, 12:25 PM · Restricted Project, Restricted Project
mibintc updated the diff for D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

This commit was reverted in 30e7ee3c4bac because a null deref error occurred in IRBuilder.h when setting strictfp attribute, see https://bugs.llvm.org/show_bug.cgi?id=44048 for information about that bug.
This patch moves setting strictfp from IRBuilder into clang/CodeGen. Also addresses some code review comments that were received after the revert. I'll add some inline comments next.

Dec 2 2019, 12:25 PM · Restricted Project, Restricted Project
mibintc added inline comments to D70451: [FPEnv] IRBuilder should not put strictfp on function definitions automatically.
Dec 2 2019, 11:11 AM · Restricted Project

Nov 20 2019

mibintc added inline comments to D70451: [FPEnv] IRBuilder should not put strictfp on function definitions automatically.
Nov 20 2019, 2:43 PM · Restricted Project

Nov 19 2019

mibintc added inline comments to D70451: [FPEnv] IRBuilder should not put strictfp on function definitions automatically.
Nov 19 2019, 12:52 PM · Restricted Project
mibintc updated the diff for D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

Here's an update in response to comments from @michele.scandale
I fixed the assertion error and added a test case
I fixed the setting of ftrapping-math in CodeGenOpts
I deleted an incorrect comment
I added a diagnostic when -fp-exception-behavior= is overridden on the command line by f[no-]trapping-math
I updated one of the test cases to work with some small modifications that will be made to IRBuilder.h

Nov 19 2019, 12:43 PM · Restricted Project, Restricted Project
mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

inline replies to Michele, will upload a new patch shortly

Nov 19 2019, 12:06 PM · Restricted Project, Restricted Project
mibintc added a comment to D70451: [FPEnv] IRBuilder should not put strictfp on function definitions automatically.

It works for me too. I need to send an update for my new test case fpconstrained.cpp
Thanks Kevin

Nov 19 2019, 11:29 AM · Restricted Project

Nov 18 2019

mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
In D62731#1750412, @kpn wrote:

Does anyone think a warning is appropriate because the new flags are exercising experimental, incomplete code in both clang and llvm? The warning would be removed when we believe the feature is complete and ready to use.

Nov 18 2019, 12:30 PM · Restricted Project, Restricted Project
mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

I added a nullptr check in IRBuilder.h and a test case to cover the segfault reported in https://bugs.llvm.org/show_bug.cgi?id=44048

Nov 18 2019, 12:12 PM · Restricted Project, Restricted Project
mibintc updated the diff for D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Nov 18 2019, 12:03 PM · Restricted Project, Restricted Project
mibintc reopened D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

Reopening since patch was reverted

Nov 18 2019, 12:03 PM · Restricted Project, Restricted Project
mibintc added inline comments to D69312: [FPEnv] Teach the IRBuilder about correct use of the strictfp attribute..
Nov 18 2019, 11:18 AM · Restricted Project
mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

Thanks I see it, I'm working on a patch. Previously there was no support for frounding-math (unimplemented). This patch enables the option. In the IR builder, there's a call to a runtime function in the exception handler which is unexpectedly null. I start by adding a null pointer check.

Had a crash on valid here for days, let's revert and then get a fix when recommitting. I'll respond to the thread when reverting. Thanks :)

Nov 18 2019, 11:08 AM · Restricted Project, Restricted Project
mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

The incorrect code is actually in the IRBuilder which is part of a different patch...

Nov 18 2019, 10:59 AM · Restricted Project, Restricted Project
mibintc added a comment to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

Thanks I see it, I'm working on a patch. Previously there was no support for frounding-math (unimplemented). This patch enables the option. In the IR builder, there's a call to a runtime function in the exception handler which is unexpectedly null. I start by adding a null pointer check.

Nov 18 2019, 6:58 AM · Restricted Project, Restricted Project

Nov 11 2019

mibintc closed D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.

Don't know why the commit id didn't get linked when I pushed the change. Here's the closure info:

Nov 11 2019, 10:49 AM · Restricted Project, Restricted Project