Page MenuHomePhabricator

mibintc (Melanie Blower)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 14 2016, 12:37 PM (165 w, 4 d)

Recent Activity

Yesterday

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.

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

Thu, Jan 16

mibintc added a reviewer for D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level: anemet.
Thu, Jan 16, 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.
Thu, Jan 16, 6:42 AM · Restricted Project, Restricted Project

Fri, Dec 20

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
}}

Fri, Dec 20, 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

Nov 8 2019

mibintc committed rGd0b3e7317504: Revert "Reapply "Fix crash on switch conditions of non-integer types in… (authored by mibintc).
Revert "Reapply "Fix crash on switch conditions of non-integer types in…
Nov 8 2019, 2:25 PM
mibintc added a reverting change for rG759948467ea3: Reapply "Fix crash on switch conditions of non-integer types in templates": rGd0b3e7317504: Revert "Reapply "Fix crash on switch conditions of non-integer types in….
Nov 8 2019, 2:25 PM
mibintc committed rG759948467ea3: Reapply "Fix crash on switch conditions of non-integer types in templates" (authored by mibintc).
Reapply "Fix crash on switch conditions of non-integer types in templates"
Nov 8 2019, 10:17 AM
mibintc closed D69950: Reapply "Fix crash on switch conditions of non-integer types in templates".
Nov 8 2019, 10:17 AM · Restricted Project

Nov 7 2019

mibintc committed rGaf57dbf12e54: Add support for options -frounding-math, ftrapping-math, -ffp-model=, and -ffp… (authored by mibintc).
Add support for options -frounding-math, ftrapping-math, -ffp-model=, and -ffp…
Nov 7 2019, 7:32 AM

Nov 5 2019

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

When doing final testing before commit, I used the Debug build with assertions enabled and found that a couple tests failed with assertion in Clang.cpp. I made some modifications there so the assert wouldn't be triggered. Also I fixed some spelling errors in the comments. (option name misspelled: missing the first f)

Nov 5 2019, 1:52 PM · Restricted Project, Restricted Project

Nov 1 2019

mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Nov 1 2019, 2:05 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.

Made a couple functions static per @rjmccall request

Nov 1 2019, 2:04 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.

Recoded ToConstrainedRoundingMD and ToConstrainedExceptionMD as requested by @rjmccall

Nov 1 2019, 1:45 PM · Restricted Project, Restricted Project
mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Nov 1 2019, 9:58 AM · 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.

Respond to recent code review from @rjmccall ; I modified the test cases and added functions for translating between the LangOptions enumeration and llvm enumeration for rounding-mode and exception-behavior. I wasn't able to use BooleanFFlag because at the moment that is only usable for unsupported options.

Nov 1 2019, 9:58 AM · Restricted Project, Restricted Project

Oct 31 2019

mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Oct 31 2019, 12:15 PM · Restricted Project, Restricted Project
mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Oct 31 2019, 8:28 AM · 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.

I followed up on some code review remarks from @rjmccall. I dropped the CODEGEN option and fixed some code formatting. I changed the spelling of the enumeration values for RoundingMode and ExceptionMode to match those proposed in https://reviews.llvm.org/D65994

Oct 31 2019, 8:28 AM · Restricted Project, Restricted Project

Oct 29 2019

mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Oct 29 2019, 12:54 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.

Is the exception-strictness of -frounding-math actually considered to be specified behavior, or is it just a consequence of the current implementation? There are definitely some optimizations that we can't do under strict FP exceptions that we can still do in principle while respecting a dynamic FP rounding mode; for example, the rounding mode can only be changed by a call (or inline assembly), so you can still reorder FP operations around "lesser" side effects, like stores. We can document it even if it's not required behavior, but we should be clear about what it is.

I had thought that it was intended behavior, but I re-checked my notes and realize I was wrong about that. So I've changed the document and the driver, and updated the test. Thanks again for your careful reading.

My suggested wording started with a sentence briefly summarizing what the option did that I think you accidentally dropped.

Yes, it's there now.

Oct 29 2019, 8:16 AM · 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.

In response to comments from @rjmccall I inserted into the UsersManual the one-line summary of frounding-math that had been omitted and changed the semantics of frounding-math to not also set exception-behavior to strict

Oct 29 2019, 8:16 AM · Restricted Project, Restricted Project

Oct 28 2019

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

I adopted the language that @rjmccall recommended for documenting frounding-math., also adding a sentence to describe effects on exception behavior control.

Oct 28 2019, 1:19 PM · Restricted Project, Restricted Project

Oct 20 2019

mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Oct 20 2019, 6:32 PM · Restricted Project, Restricted Project

Oct 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.

Ping. Hoping for code review so I can move this forward. Affirmative or negative, please let me know. Thank you! --Melanie

Oct 18 2019, 10:11 AM · Restricted Project, Restricted Project

Oct 10 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 looked over the codegen testcase fpconstrained.c and it looks pretty good, so i think this is ready for your review comments. I'll be off the grid for a couple days but looking forward to receiving your feedback.

Oct 10 2019, 8:12 AM · Restricted Project, Restricted Project

Oct 9 2019

mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Oct 9 2019, 1:19 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.

I added a new test case fp-model.c to test RenderFloatingPointOptions, I also fixed a few issues that I spotted while working through this test case. I responded to couple documentation comments from @rjmccall I still owe a more deluxe version of the test fp-constrained.c to be sure all the option values come through as expected

Oct 9 2019, 1:12 PM · Restricted Project, Restricted Project

Oct 8 2019

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

I added a test case to show the warning diagnostics when options conflicting with fp-model are provided. I fixed a couple bugs in RenderFloatingPointOptions when issueing diagnostics. still owe a test case showing how the fp-model, rounding, and trapping options are rendered by the Driver for cc1

Oct 8 2019, 2:05 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.

clean up some dead code

Oct 8 2019, 11:44 AM · Restricted Project, Restricted Project
mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Oct 8 2019, 11:36 AM · Restricted Project, Restricted Project
mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Oct 8 2019, 11:35 AM · 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.

I made a couple wording changes suggested by @rjmccall

Oct 8 2019, 11:25 AM · Restricted Project, Restricted Project

Oct 3 2019

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

In the previous review, @rjmccall asked me to redo the existing floating point option documentation before submitting this patch. I got the floating point documentation update committed, and I've worked on this patch more to get the floating point "render options" checking implemented. This patch needs more test cases, and there might be a bug or 2 in the "render options" checking. I wanted to show you this work especially to get your reaction to the changes to the floating point options

Oct 3 2019, 2:16 PM · Restricted Project, Restricted Project

Sep 17 2019

mibintc updated the diff for D67517: Create UsersManual section entitled 'Controlling Floating Point Behavior'.

fix underline

Sep 17 2019, 8:19 AM · Restricted Project
mibintc added a comment to D67517: Create UsersManual section entitled 'Controlling Floating Point Behavior'.

ready to commit? anything else needed?

Sep 17 2019, 7:26 AM · Restricted Project

Sep 16 2019

mibintc added a comment to D67517: Create UsersManual section entitled 'Controlling Floating Point Behavior'.

@rjmccall I don't have commit rights, if you think it's ready can you submit? thanks for all

Sep 16 2019, 1:05 PM · Restricted Project
mibintc updated the diff for D67517: Create UsersManual section entitled 'Controlling Floating Point Behavior'.

Respond to @rjmccall 's review

Sep 16 2019, 12:47 PM · Restricted Project
mibintc updated the diff for D67517: Create UsersManual section entitled 'Controlling Floating Point Behavior'.
Sep 16 2019, 10:44 AM · Restricted Project
mibintc added inline comments to D67517: Create UsersManual section entitled 'Controlling Floating Point Behavior'.
Sep 16 2019, 10:40 AM · Restricted Project

Sep 13 2019

mibintc added reviewers for D67517: Create UsersManual section entitled 'Controlling Floating Point Behavior': andrew.w.kaylor, kpn.
Sep 13 2019, 7:01 AM · Restricted Project
mibintc added inline comments to D67517: Create UsersManual section entitled 'Controlling Floating Point Behavior'.
Sep 13 2019, 6:31 AM · Restricted Project

Sep 12 2019

mibintc added inline comments to D67517: Create UsersManual section entitled 'Controlling Floating Point Behavior'.
Sep 12 2019, 1:36 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.

Hmm, you know, there are enough different FP options that I think we should probably split them all out into their own section in the manual instead of just listing them under "code generation". That will also give us an obvious place to describe the basic model, i.e. all the stuff about it mostly coming down to different strictness and exception models. Could you prepare a patch that *just* does that reorganization without adding any new features, and then we can add the new options on top of that?

Sep 12 2019, 1:26 PM · Restricted Project, Restricted Project
mibintc created D67517: Create UsersManual section entitled 'Controlling Floating Point Behavior'.
Sep 12 2019, 1:25 PM · Restricted Project
mibintc retitled D67517: Create UsersManual section entitled 'Controlling Floating Point Behavior' from Create UsersManual section entitled 'Controlling Floating Poit Behavior' to Create UsersManual section entitled 'Controlling Floating Point Behavior'.
Sep 12 2019, 1:25 PM · Restricted Project

Sep 9 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.

Hmm, you know, there are enough different FP options that I think we should probably split them all out into their own section in the manual instead of just listing them under "code generation". That will also give us an obvious place to describe the basic model, i.e. all the stuff about it mostly coming down to different strictness and exception models. Could you prepare a patch that *just* does that reorganization without adding any new features, and then we can add the new options on top of that?

Sep 9 2019, 3:32 PM · Restricted Project, Restricted Project
mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Sep 9 2019, 1:29 PM · Restricted Project, Restricted Project
mibintc retitled D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior from [RFC] Add support for options -frounding-math -fp-model= and -fp-exception-behavior= : specify floating point behavior to [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior.
Sep 9 2019, 8:56 AM · 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.

Addressed comments from @rjmccall and @andrew.w.kaylor. Added -frounding-math option and -ffp-exception-behavior= option. Dropped -ffp-speculation= option. Added details about how the new options conflict with existing floating point options. Rewrote
RenderFloatingPointOptions using pseudo code to show how option conflicts will be detected.

Sep 9 2019, 8:51 AM · Restricted Project, Restricted Project
mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Sep 9 2019, 8:39 AM · Restricted Project, Restricted Project

Aug 27 2019

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

I made a couple changes to the UserManual in response to @rjmccall review

Aug 27 2019, 10:13 AM · Restricted Project, Restricted Project
mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Aug 27 2019, 10:11 AM · Restricted Project, Restricted Project

Aug 22 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.

ping, looking for a code review, especially from front end folks. thank you!

Aug 22 2019, 1:00 PM · Restricted Project, Restricted Project

Aug 16 2019

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

I addressed some comments from @kpn: I corrected the documentation formatting and added some details, and used Diag instead of llvm_unreachable.

Aug 16 2019, 1:56 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 an inline reply and unmarked some things that had been inadvertently marked done.

Aug 16 2019, 1:32 PM · Restricted Project, Restricted Project

Aug 15 2019

mibintc added inline comments to D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior.
Aug 15 2019, 1:07 PM · Restricted Project, Restricted Project
mibintc added reviewers for D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior: rsmith, rjmccall.
Aug 15 2019, 6:46 AM · Restricted Project, Restricted Project

Aug 14 2019

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

I added documentation for the new floating point options into clang/docs

Aug 14 2019, 1:27 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.

Here's a summary of how the rm and eb options are set

Aug 14 2019, 8:48 AM · Restricted Project, Restricted Project

Aug 9 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'm not entirely caught up on this review. I've only read the most recent comments, but I think I've got enough context to comment on the metadata arguments.

Based only on the fp-model command line options, the front end should only ever use "round.dynamic" (for strict mode) or "round.tonearest" (where full fenv access isn't permitted but we want to enable strict

Aug 9 2019, 8:36 AM · Restricted Project, Restricted Project

Aug 7 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.

Compared to the 2nd revision, this patch moves all the changes into clang, removing the FPState file.

In the summary, I've copied information from Microsoft about the fp-model options into the differential Summary, as requested by Kevin, to create a legacy for future maintainers in case that information disappears from msdn.

The the documentation should be documentation, it should reside somewhere in clang/docs/.

Got it thanks

Aug 7 2019, 2:40 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.
In D62731#1603030, @kpn wrote:

I actually don't have much of an opinion on what the command line argument form should be. It may be helpful for it to be the same as one of the commonly deployed compilers. The worst I think would be pretty close but with subtle differences. So if it can be made to work like Intel's compiler I'm fine with that. But I'm hoping that more people in the community chime in since having a consensus would be best. Personally, I'm not yet giving any final sign-offs to tickets since I don't think I've been here long enough.

As far as the rounding metadata argument, the language reference says this:

For values other than “round.dynamic” optimization passes may assume that the actual runtime rounding mode (as defined in a target-specific manner) matches the specified rounding mode, but this is not guaranteed. Using a specific non-dynamic rounding mode which does not match the actual rounding mode at runtime results in undefined behavior.

Be aware that currently neither of the metadata arguments does anything. They get dropped when llvm reaches the SelectionDAG. And none of the optimization passes that run before that know anything about constrained intrinsics at all. This means they treat code that has them conservatively. Preserving and using that metadata in the optimization passes and getting it down and used by the MI layer is planned but hasn't happened yet. So the full set of arguments may not make sense yet, but an on/off switch for strict mode hopefully does.

Aug 7 2019, 2:39 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.
In D62731#1601408, @kpn wrote:

I think it would be convenient to have an "unset" setting for the different constrained modes, otherwise you need a boolean that says "no value was provided for this option". But i'm a frontend person so I may need to have my attitude adjusted.

What "different constrained modes"? The IRBuilder is either in constrained mode or it isn't. In constrained mode the exception behavior and rounding mode both have defaults, and those defaults can be changed individually without affecting the other setting. The current defaults can also be retrieved if you need something for a function call where you don't want to change it but need an argument anyway. When do you need this "no value provided" setting?

Oh, I'd check the tools/clang/CODE_OWNERS.txt file and add additional appropriate reviewers. Perhaps John McCall and Richard Smith? I don't know who has opinions on how command line options should be handled.

Do we want the Unix driver to be compatible with gcc? Maybe, maybe not. Opinions, anyone?

Yes we need opinions on this.

The documentation request from lebedev.ri isn't in this ticket yet.

I'm not yet sure what I need for this, the requested documentation is still missing.

Also, for future historical research purposes I'd cut and paste the relevant portions of outside web pages (like intel.com's) and post them somewhere llvm-ish where they are findable. This ticket, for example, is a good place. Web sites gets reorganized or vanish in full or in part. It's helpful to have some insulation from that over time. I've had to fix compiler bugs that actually were 25 years old before. Yes, 25 years old. Being able to do the research is very helpful.

Oh, it may be useful to know that constrained floating point and the FastMathFlags are not mutually exclusive. I don't know if that matters here or not, but you did mention FastMathFlags.

Yes they aren't mutually exclusive. One of the fp-model options implies enabling fast math.

Aug 7 2019, 2:38 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.

Compared to the 2nd revision, this patch moves all the changes into clang, removing the FPState file.

Aug 7 2019, 2:32 PM · Restricted Project, Restricted Project

Jul 26 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.

Thanks for your review >>! In D62731#1601408, @kpn wrote:

I think it would be convenient to have an "unset" setting for the different constrained modes, otherwise you need a boolean that says "no value was provided for this option". But i'm a frontend person so I may need to have my attitude adjusted.

What "different constrained modes"? The IRBuilder is either in constrained mode or it isn't. In constrained mode the exception behavior and rounding mode both have defaults, and those defaults can be changed individually without affecting the other setting. The current defaults can also be retrieved if you need something for a function call where you don't want to change it but need an argument anyway. When do you need this "no value provided" setting?

I'm going to rewrite this

Oh, I'd check the tools/clang/CODE_OWNERS.txt file and add additional appropriate reviewers. Perhaps John McCall and Richard Smith? I don't know who has opinions on how command line options should be handled.

I'd like to fix it more before I add more reviewers

Do we want the Unix driver to be compatible with gcc? Maybe, maybe not. Opinions, anyone?

Oh, I think you mean something more like the gnuish -fno-except or maybe -fp-model=no-except? instead of -fp-model=except- ok we can get that sorted.

The documentation request from lebedev.ri isn't in this ticket yet.

Also, for future historical research purposes I'd cut and paste the relevant portions of outside web pages (like intel.com's) and post them somewhere llvm-ish where they are findable. This ticket, for example, is a good place. Web sites gets reorganized or vanish in full or in part. It's helpful to have some insulation from that over time. I've had to fix compiler bugs that actually were 25 years old before. Yes, 25 years old. Being able to do the research is very helpful.

That's a good idea thanks

Oh, it may be useful to know that constrained floating point and the FastMathFlags are not mutually exclusive. I don't know if that matters here or not, but you did mention FastMathFlags.

Yes i'm not sure how the fast math command line optoins should interact with the fp-model options, i'll have to dig into that.

Jul 26 2019, 12:05 PM · Restricted Project, Restricted Project

Jul 25 2019

mibintc abandoned D26636: patch for llvm/clang bug 25965, suppress warning diagnostic on float-to-bool conversion when in condition context.
Jul 25 2019, 9:18 AM · Restricted Project