[PATCH] D30662: Update clang filtering for mxcsr

andrew.w.kaylor created this revision. This patch updates the clang function that filters the mxcsr register name to recognize that mxcsr is being split into two pseduo-registers that model the control and status bits separately. Repository: rL LLVM https://reviews.llvm.org/D30662 Files:

[PATCH] D30662: Update clang filtering for mxcsr

andrew.w.kaylor added a comment. In https://reviews.llvm.org/D30662#693559, @ahatanak wrote: > Is it possible to add a test case (possibly in CodeGen)? This is covered by CodeGen/ms-inline-asm.c, which Reid added after I broke this adding the mxcsr register a couple of weeks ago.

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor created this revision. This patch adds a hack to clang's emitPointerArithmetic() function to get it to emit an inttoptr instruction rather than a GEP with a null base pointer when the 'p = nullptr + n' idiom is used to convert a pointer-sized integer to a pointer. This idiom

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor added a comment. I'm not sure why the svn attributes got attached to the file I added. I'll remove them before committing. https://reviews.llvm.org/D37042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38060: Remove offset size check in nullptr arithmetic handling

andrew.w.kaylor created this revision. This patch amends the changes that were committed from https://reviews.llvm.org/D37042 to remove the offset size check in the idiom identification routine. That check was behaving inconsistently (from target to target) because the offset was implicitly

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor added a comment. This is breaking buildbots for 32-bit targets because the offset in 'nullptr + int8_t_N' is being implicitly cast to an int. That makes the sizeof(offset) == sizeof(ptr) check turn out differently than my tests were assuming. I can get the buildbots green

[PATCH] D38060: Remove offset size check in nullptr arithmetic handling

andrew.w.kaylor closed this revision. andrew.w.kaylor added a comment. This was committed as r313784. I put the wrong differential revision number in the comment for that check-in. https://reviews.llvm.org/D38060 ___ cfe-commits mailing list

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6031 InGroup; +def ext_gnu_null_ptr_arith : Extension< + "arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension">, efriedma

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6031 InGroup; +def ext_gnu_null_ptr_arith : Extension< + "arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension">, efriedma

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor added a comment. Does anything else need to be done for this to be ready to land? https://reviews.llvm.org/D37042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor updated this revision to Diff 115262. andrew.w.kaylor added a comment. -Changed GNU idiom from extension to warning. -Updated to ToT. https://reviews.llvm.org/D37042 Files: include/clang/AST/Expr.h include/clang/Basic/DiagnosticGroups.td

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor updated this revision to Diff 113134. andrew.w.kaylor added a comment. Added warnings for null pointer arithmetic. https://reviews.llvm.org/D37042 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/CodeGen/CGExprScalar.cpp

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor updated this revision to Diff 113343. andrew.w.kaylor added a comment. Fixed value-dependent argument in isNullPointerConstant checks. Added check for C++ zero offset in subtraction. Added value-dependent test cases. https://reviews.llvm.org/D37042 Files:

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor added inline comments. Comment at: lib/AST/Expr.cpp:1857 + if (!PExp->IgnoreParenCasts() + ->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNull)) +return false; rsmith wrote: > If we get here with a value-dependent

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor updated this revision to Diff 113311. andrew.w.kaylor added a comment. Refactored the GNU idiom check to be shared by Sema and CodeGen. Refined the checks so that nullptr+0 doesn't warn in C++. Added the zero offset qualification in the warning produced with C++.

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor added a comment. Ping. https://reviews.llvm.org/D37042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor added a comment. My intention here was to make this as strict/limited as possible while still handling the cases of interest. There are two different implementations I want to handle. You can see the first variation in the __BPTR_ALIGN definition being added here:

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor added a comment. In https://reviews.llvm.org/D37042#850676, @hfinkel wrote: > I'd really like to see this done in some way such that we can issue a warning > along with generating well-defined code. The warning can specifically state > that the code is using an extension, etc.

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor added inline comments. Comment at: lib/Sema/SemaExpr.cpp:8808 +Context.getTypeSize(pointerType) == +Context.getTypeSize(IExp->getType())) + IsGNUIdiom = true; efriedma wrote: > rsmith wrote: > > andrew.w.kaylor wrote: >

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6032 +def ext_gnu_null_ptr_arith : Extension< + "inttoptr casting using arithmetic on a null pointer is a GNU extension">, + InGroup; rsmith wrote: > efriedma wrote:

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

andrew.w.kaylor added inline comments. Comment at: lib/Sema/SemaExpr.cpp:8808 +Context.getTypeSize(pointerType) == +Context.getTypeSize(IExp->getType())) + IsGNUIdiom = true; efriedma wrote: > Please make sure you use exactly the same

[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers

andrew.w.kaylor added a comment. I hope I'm not coming across as too argumentative here. I don't really have strong feelings about this function pointer type patch and ultimately I see that you are right, but the objections you are raising here would equally apply to what I'm working on with

[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers

andrew.w.kaylor added a comment. Fair enough. FYI, I've spent most of the day poking at the IR linker and I've found all sorts of ways that I can get it to make a complete mess of structure types and pointers to them, including some simple cases where it will mess it up in different ways

[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers

andrew.w.kaylor added a comment. I've talked to Olga about this, and I think we have a way to handle the problem she was working on without this change. However, I think this change is worth considering anyway. The test case shows an example where clang is clearly not producing the output it

[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers

andrew.w.kaylor added a comment. The LLVM linker also seems to have a bunch of problems with resolving pointer types in structures. I'm looking into a couple of those now. I am aware of the opaque pointer effort, though as you say it seems to be stalled. I agree that there aren't a lot of

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

andrew.w.kaylor added a comment. Craig Topper also raised some concerns with me (in person, his desk is right by mine) about the potential effect this might have on code size. He tells me that IRBuilder calls are intended to always be inlined and if we grow the implementation of these

[PATCH] D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR

andrew.w.kaylor added a comment. In https://reviews.llvm.org/D45616#1290897, @nastafev wrote: > > can trigger arbitrary floating-point exceptions anywhere in your code > > I believe this statement reflects the current state of many compilers on the > market, I guess I just don't see the

[PATCH] D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR

andrew.w.kaylor added a comment. Agreed. Reverting this patch wouldn't move us forward on constrained FP handling. What I'm saying (and what I think @nastafev is saying) is that this patch is taking a built-in that allows the user to express specific signalling behavior and throwing away that

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

andrew.w.kaylor added a comment. In https://reviews.llvm.org/D53157#1291978, @cameron.mcinally wrote: > In https://reviews.llvm.org/D53157#1291964, @kpn wrote: > > > I don't expect anyone would need to switch between constrained and regular > > floating point at an instruction level of

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

andrew.w.kaylor added a comment. In https://reviews.llvm.org/D53157#1304895, @cameron.mcinally wrote: > The problem I was missing is when a FENV_ACCESS=OFF operation, like a FDIV, > is hoisted into a FENV_ACCESS=ON region. I see it now, but still think that > forcing FENV_ACCESS=OFF operations

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

andrew.w.kaylor added a comment. In https://reviews.llvm.org/D53157#1302724, @uweigand wrote: > A couple of comments on the previous discussion: > > 1. Instead of defining a new command line option, I'd prefer to use the > existing options -frounding-math and -ftrapping-math to set the default

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

andrew.w.kaylor added inline comments. Comment at: include/llvm/IR/IRBuilder.h:234 + /// Set the exception handling to be used with constrained floating point + void setDefaultConstrainedExcept(MDNode *NewExcept) { +DefaultConstrainedExcept = NewExcept;

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

andrew.w.kaylor added a comment. In this review (https://reviews.llvm.org/D6260) @rsmith mentions that this should also have an effect on name mangling. What will this do if the user calls a library function that expects a long double? What does gcc do in that case? Repository: rC Clang

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

andrew.w.kaylor added a comment. In D64067#1568895 , @hfinkel wrote: > One thing to realize about these flags is that they're ABI-altering flags. If > the user provides the flag to alter the platform defaults, this only works if > the user also ensures

[PATCH] D66092: [CodeGen] Generate constrained fp intrinsics depending on FPOptions

andrew.w.kaylor added a comment. In D66092#1632642 , @sepavloff wrote: > - What is the issue with moving `a = b/c`? If it moves ahead of `if` > statement it seems OK, because the rounding mode is the same in that point. > It cannot be moved inside the

[PATCH] D66092: [CodeGen] Generate constrained fp intrinsics depending on FPOptions

andrew.w.kaylor added a comment. It took some digging, but I finally found the e-mail thread where we initially agreed that we can't mix constrained FP intrinsics and non-constrained FP operations within a function. Here it is: http://lists.llvm.org/pipermail/cfe-dev/2017-August/055325.html

[PATCH] D66092: [CodeGen] Generate constrained fp intrinsics depending on FPOptions

andrew.w.kaylor added a comment. In D66092#1630997 , @sepavloff wrote: > Replacement of floating point operations with constrained intrinsics seems > more an optimization helper then a semantic requirement. IR where constrained > operations are mixed

[PATCH] D65997: Add options rounding and exceptions to pragma fp

andrew.w.kaylor added inline comments. Comment at: clang/docs/LanguageExtensions.rst:3127 The pragma can take three values: ``on``, ``fast`` and ``off``. The ``on`` option is identical to using ``#pragma STDC FP_CONTRACT(ON)`` and it allows This part of the

[PATCH] D66092: [CodeGen] Generate constrained fp intrinsics depending on FPOptions

andrew.w.kaylor added a comment. In D66092#1627339 , @sepavloff wrote: > In D66092#1625380 , @kpn wrote: > > > Also, if any constrained intrinsics are used in a function then the entire > > function needs to be

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

andrew.w.kaylor added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:126 + case LangOptions::FPM_Precise: + case LangOptions::FPM_Fast: +break; mibintc wrote: > mibintc wrote: > > kpn wrote: > > > Wait, so "fast" and "precise" are the

[PATCH] D65997: Add options rounding and exceptions to pragma fp

andrew.w.kaylor added inline comments. Comment at: clang/docs/LanguageExtensions.rst:3152 +rounding mode. This option is experimental; the compiler may ignore an explicit +rounding mode in some situations. + sepavloff wrote: > andrew.w.kaylor wrote: > > You

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

andrew.w.kaylor added inline comments. Comment at: clang/docs/UsersManual.rst:1305 + and ``noexcept``. Note that -fp-model=[no]except can be combined with the + other three settings for this option. Details: + rjmccall wrote: > mibintc wrote: > > rjmccall

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

andrew.w.kaylor added inline comments. Comment at: clang/docs/UsersManual.rst:1309 + + * ``precise`` Disables optimizations that are not value-safe on floating-point data, although FP contraction (FMA) is enabled. + * ``strict`` Enables precise and except, and disables

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

andrew.w.kaylor added a comment. 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"

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

andrew.w.kaylor added a comment. In D62731#1623114 , @mibintc wrote: > @andrew.w.kaylor Thanks Andy. Reminder -- in a private document you > indicated to me that -fp-speculation=safe corresponds to the maytrap setting > for the exception argument.

[PATCH] D69598: Work on cleaning up denormal mode handling

andrew.w.kaylor added a comment. I'm unclear as to the expectations surrounding this option. I suppose this is somewhat beyond the scope of the current changes, but I'm confused by clang's current behavior with regard to denormals. The -fdenromal-fp-math option causes a function attribute to

[PATCH] D69598: Work on cleaning up denormal mode handling

andrew.w.kaylor accepted this revision. andrew.w.kaylor added a comment. Thanks. I understand your direction for denormal handling now, and I'm OK with this patch apart from the remaining references to subnormal that Sanjay mentioned. In D69598#1739723

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

andrew.w.kaylor added a comment. In D62731#1782762 , @rjmccall wrote: > Currently we emit a warning if you use `-frounding-math`, saying that the > option is ignored. That should have alerted users that they're not getting > the correct behavior now.

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

andrew.w.kaylor added a comment. In D62731#1782912 , @rjmccall wrote: > Hmm. The target-specific intrinsics thing does concern me, since I assume > many targets have a bunch of vector intrinsics that may be sensitive to > rounding. Do we have an idea

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

andrew.w.kaylor added a comment. In D62731#1784225 , @rjmccall wrote: > ... The big problem is that intrinsic calls are not arbitrary code: the vast > majority of intrinsics (e.g. all the ARM vector intrinsics, many of which can > be floating-point) are

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

andrew.w.kaylor added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:444 +def warn_drv_experimental_fp_control_incomplete_opt : Warning< + "Support for floating point control option %0 is incomplete and experimental">,

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

andrew.w.kaylor added a comment. In D62731#1788838 , @rupprecht wrote: > 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 >

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

andrew.w.kaylor added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:444 +def warn_drv_experimental_fp_control_incomplete_opt : Warning< + "Support for floating point control option %0 is incomplete and experimental">,

[PATCH] D69272: Restricted variant of '#pragma STDC FENV_ACCESS'

andrew.w.kaylor added a comment. Thanks for the patch! I don't have time to review this in detail this week, but I'm very happy to see this functionality. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:890 + "cannot apply to inline functions, ignoring pragma">,

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

andrew.w.kaylor added inline comments. Comment at: llvm/include/llvm/IR/IRBuilder.h:262 Function *F = BB->getParent(); -if (!F->hasFnAttribute(Attribute::StrictFP)) { +if (F && !F->hasFnAttribute(Attribute::StrictFP)) { F->addFnAttr(Attribute::StrictFP);

[PATCH] D69978: Separately track input and output denormal mode

andrew.w.kaylor added inline comments. Comment at: llvm/docs/LangRef.rst:1822 ``"denorm-fp-mode"`` + This indicates the subnormal handling that may be assumed for the I don't like the definition of this attribute. It's not reader-friendly. The

[PATCH] D71635: [clang] Rename -frounding-math to -fexperimental-rounding-math and add -frounding-math back as a gcc-compat arg.

andrew.w.kaylor added a comment. In D71635#1790611 , @MaskRay wrote: > Before: > > % clang -frounding-math -fsyntax-only -x c /dev/null > clang-10: warning: Support for floating point control option frounding-math > is incomplete and experimental

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

andrew.w.kaylor added a comment. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74436/new/ https://reviews.llvm.org/D74436 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74500: clang: Treat ieee mode as the default for denormal-fp-math

andrew.w.kaylor accepted this revision. andrew.w.kaylor added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74500/new/ https://reviews.llvm.org/D74500 ___ cfe-commits mailing

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

andrew.w.kaylor added a comment. 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. Consider

[PATCH] D72906: [X86] Improve X86 cmpps/cmppd/cmpss/cmpsd intrinsics with strictfp

andrew.w.kaylor accepted this revision. andrew.w.kaylor added a comment. This revision is now accepted and ready to land. lgtm I have a couple of comments, but nothing that couldn't be addressed in a later patch. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:12363 +Cmp

[PATCH] D69978: Separately track input and output denormal mode

andrew.w.kaylor added inline comments. Comment at: llvm/docs/LangRef.rst:1829 + operations. The second indicates the handling of denormal inputs to + floating point instructions. + Based on the changes below, if the second value is omitted the input mode

[PATCH] D69978: Separately track input and output denormal mode

andrew.w.kaylor accepted this revision. andrew.w.kaylor added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69978/new/ https://reviews.llvm.org/D69978 ___ cfe-commits mailing

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

andrew.w.kaylor added a comment. In D72675#1839492 , @wristow wrote: > 1. Should we enable FMA "by default" at (for example) '-O2'? We recently introduced a new option "-ffp-model=[precise|fast|strict], which is supposed to serve as an umbrella option

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

andrew.w.kaylor added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2792 if (!MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros && - !TrappingMath) + !TrappingMath && !(FPContract.equals("off") || FPContract.equals("on")))

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

andrew.w.kaylor added a subscriber: scanon. andrew.w.kaylor added a comment. In D74436#1875320 , @mibintc wrote: > However you are right we don't want the frontend to create FMA when > optimizations are disabled -O0 I've been discussing this with

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

andrew.w.kaylor added a comment. In D74436#1875315 , @nemanjai wrote: > > You're right, -O0 shouldn't generate FMA. I'm preparing to revert this now > > -- just verifying the build. > > Perhaps this should be > `off` with no optimization > `on` with

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

andrew.w.kaylor added a subscriber: MaskRay. andrew.w.kaylor added a comment. In D74436#1875386 , @thakis wrote: > The revert of this breaks tests everywhere, as far as I can tell. It looks like something strange happened with the revert: > clang-11:

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

andrew.w.kaylor added inline comments. Comment at: clang/docs/UsersManual.rst:1388 - * ``precise`` Disables optimizations that are not value-safe on floating-point data, although FP contraction (FMA) is enabled (``-ffp-contract=fast``). This is the default behavior. *

[PATCH] D69878: Consoldiate internal denormal flushing controls

andrew.w.kaylor added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + cameron.mcinally wrote: > andrew.w.kaylor wrote: >

[PATCH] D69878: Consoldiate internal denormal flushing controls

andrew.w.kaylor accepted this revision. andrew.w.kaylor added a comment. This revision is now accepted and ready to land. I don't know if there were other reviewers who haven't commented on how you addressed their concerns, but this looks good to me. Thanks for taking the time to improve our

[PATCH] D72820: [FPEnv] Add pragma FP_CONTRACT support under strict FP.

andrew.w.kaylor added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3436 +} + } + cameron.mcinally wrote: > andrew.w.kaylor wrote: > > cameron.mcinally wrote: > > > cameron.mcinally wrote: > > > > cameron.mcinally wrote: > > > > > I don't

[PATCH] D72820: [FPEnv] Add pragma FP_CONTRACT support under strict FP.

andrew.w.kaylor added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3381 + Addend->getType()), +{MulOp0, MulOp1, Addend, MulOp->getOperand(2), MulOp->getOperand(3)}); + else You shouldn't just assume that

[PATCH] D69878: Consoldiate internal denormal flushing controls

andrew.w.kaylor added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2311 + bool TrappingMath = true; // overriden by ffp-exception-behavior? bool RoundingFPMath = false; arsenm wrote: >

[PATCH] D72820: [FPEnv] Add pragma FP_CONTRACT support under strict FP.

andrew.w.kaylor added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3436 +} + } + cameron.mcinally wrote: > cameron.mcinally wrote: > > cameron.mcinally wrote: > > > I don't think it's safe to fuse a FMUL and FADD if the intermediate > >

[PATCH] D69878: Consoldiate internal denormal flushing controls

andrew.w.kaylor added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + arsenm wrote: > andrew.w.kaylor wrote: > > arsenm

[PATCH] D69878: Consoldiate internal denormal flushing controls

andrew.w.kaylor added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + arsenm wrote: > andrew.w.kaylor wrote: > > Can you

[PATCH] D69878: Consoldiate internal denormal flushing controls

andrew.w.kaylor added inline comments. Comment at: llvm/docs/LangRef.rst:1837 + are present, this overrides ``"denormal-fp-math"``. Not all targets + support separately setting the denormal mode per type. + Can you document which targets do support the

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

andrew.w.kaylor added a comment. In D72675#1920309 , @lebedev.ri wrote: > I may be wrong, but i suspect those failures aren't actually due to the fact > that we pessimize optimizations with this change, but that the whole > execution > just fails. Can

[PATCH] D99675: RFC [llvm][clang] Create new intrinsic llvm.arith.fence to control FP optimization at expression level

andrew.w.kaylor added a comment. In D99675#2671924 , @efriedma wrote: >> The expression “llvm.arith.fence(a * b) + c” means that “a * b” must happen >> before “+ c” and FMA guarantees that, but to prevent later optimizations >> from unpacking the FMA

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

andrew.w.kaylor added a comment. FWIW, fp-contract=on has been the documented default for clang since version 5. https://releases.llvm.org/5.0.1/tools/clang/docs/ClangCommandLineReference.html#cmdoption-clang-ffp-contract This change just brought the behavior into conformance with the

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

andrew.w.kaylor added inline comments. Comment at: clang/test/CodeGen/ffp-model.c:4 +// RUN: | FileCheck %s \ +// RUN: --check-prefixes=CHECK,CHECK-FAST --strict-whitespace + Why did you add the strict-whitespace option? Comment at:

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

andrew.w.kaylor closed this revision. andrew.w.kaylor added a comment. Closed by commit by rGf04e387055e495e3e14570087d68e93593cf2918 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107994/new/

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

andrew.w.kaylor accepted this revision. andrew.w.kaylor added a comment. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107994/new/ https://reviews.llvm.org/D107994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D114564: Fix the use of -fno-approx-func along with -Ofast or -ffast-math

andrew.w.kaylor added a comment. Thanks for the patch! This looks mostly good. I have just a few suggestions. Could you add test cases in clang/test/Driver/clang_f_opts.c to verify that the various driver inputs get overridden in the expected way? Without such a test, this behavior is likely

[PATCH] D112094: Add support for floating-point option `ffp-eval-method` and for `pragma clang fp eval_method`.

andrew.w.kaylor added a comment. Herald added a subscriber: luke957. I agree that the pragma should also control the evaluation of constant expressions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112094/new/ https://reviews.llvm.org/D112094

[PATCH] D120395: [X86] Prohibit arithmetic operations on type `__bfloat16`

andrew.w.kaylor added a comment. In D120395#3358533 , @craig.topper wrote: > __m256bh should not have been a new type. It should have been an alias of > __m256i. We don't have load/store intrinsics for __m256bh so if you can even > get the __m256bh

[PATCH] D120395: [X86] Prohibit arithmetic operations on type `__bfloat16`

andrew.w.kaylor added a comment. In D120395#3359255 , @pengfei wrote: > I don't agree. Unlike `__fp16`, `__bf16` is simple an ARM specific type. Why is __bf16 an ARM-specific type? It's a type that describes a floating point value with a specific

[PATCH] D121410: Have cpu-specific variants set 'tune-cpu' as an optimization hint

andrew.w.kaylor accepted this revision. andrew.w.kaylor added a comment. This revision is now accepted and ready to land. This looks good to me. Thanks for the patch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121410/new/ https://reviews.llvm.org/D121410

[PATCH] D121122: Set FLT_EVAL_METHOD to -1 when fast-math is enabled.

andrew.w.kaylor added a comment. The fix for the eval_method crash should be moved to a separate patch. Otherwise, this looks good. I have only minor comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1600 +} else { + OS << getTUFPEvalMethod(); + //

[PATCH] D121410: Have cpu-specific variants set 'tune-cpu' as an optimization hint

andrew.w.kaylor added a comment. This example illustrates the problem this patch intends to fix: https://godbolt.org/z/j445sxPMc For Intel microarchitectures before Skylake, the LLVM cost model says that vector fsqrt is slow, so if fast-math is enabled, we'll use an approximation rather than

[PATCH] D121410: Have cpu-specific variants set 'tune-cpu' as an optimization hint

andrew.w.kaylor added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2067 + // favor this processor. + TuneCPU = SD->getCPUName(GD.getMultiVersionIndex())->getName(); +} Unfortunately, I don't think it's this easy. The list of

[PATCH] D120395: [X86] Prohibit arithmatic operations on type `__bfloat16`

andrew.w.kaylor added a comment. In D120395#3340953 , @craig.topper wrote: > These intrinsics pre-date the existence of the bfloat type in LLVM. To use > bfloat we have to make __bf16 a legal type in C. This means we need to > support loads, stores,

[PATCH] D120395: [X86] Prohibit arithmetic operations on type `__bfloat16`

andrew.w.kaylor added a comment. In D120395#3344890 , @pengfei wrote: > Disscussed with GCC folks. We think it's better to use the same way as > D120411 that replacing it with short int. Which GCC folks did you

[PATCH] D120411: [X86] Replace __m[128|256|512]bh with __m[128|256|512]i and mark the former deprecated

andrew.w.kaylor requested changes to this revision. andrew.w.kaylor added a comment. This revision now requires changes to proceed. Replacing `__m128bh` with `__m128i` does not prevent arithmetic operations on the type. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D120395: [X86] Prohibit arithmetic operations on type `__bfloat16`

andrew.w.kaylor added inline comments. Comment at: clang/lib/Headers/avx512bf16intrin.h:37 ///and fraction field is extended to 23 bits. -static __inline__ float __DEFAULT_FN_ATTRS _mm_cvtsbh_ss(__bfloat16 __A) { +static __inline__ float __DEFAULT_FN_ATTRS

[PATCH] D120395: [X86] Prohibit arithmatic operations on type `__bfloat16`

andrew.w.kaylor added a comment. In D120395#3340496 , @pengfei wrote: > Update LangRef. We use `i16` type to represent bfloat16. Why are we using i16 to represent bfloat16? The bfloat type is better. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D120395: [X86] Prohibit arithmetic operations on type `__bfloat16`

andrew.w.kaylor added a comment. Herald added a project: All. In D120395#3346591 , @scanon wrote: > There's a lot of churn around proposed "solutions" on this and related PR, > but not a very clear analysis of what the problem we're trying to solve is.

[PATCH] D120395: [X86] Prohibit arithmetic operations on type `__bfloat16`

andrew.w.kaylor added a comment. In D120395#3356355 , @pengfei wrote: > Good question! This is actually the scope of ABI. Unfortunately, we don't > have the BF16 ABI at the present. We can't assume what are the physical > registers the arguments been

[PATCH] D122155: Add warning when eval-method is set in the presence of value unsafe floating-point calculations.

andrew.w.kaylor added inline comments. Comment at: clang/include/clang/Basic/DiagnosticFrontendKinds.td:50-53 +def warn_eval_method_setting_via_option_in_value_unsafe_context : Warning< +"setting the eval method via '-ffp-eval-method' has not effect when numeric " +

  1   2   >