[PATCH] D138109: [clang] Fix -fp-model={strict|precise} to disable -fapprox-func

2022-11-16 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/test/Driver/fp-model.c:162 +// CHECK-FASTMATH-FPM-STRICT-NOT: "-ffast-math" +// CHECK-FASTMATH-FPM-STRICT-NOT: "-ffinite-math-only" Shouldn't be worth adding a rule for -ffast-math and ffp-model=fast (even if it

[PATCH] D137578: Fix 'unsafe-math-optimizations' flag dependencies

2022-11-11 Thread Zahira Ammarguellat via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG91628f0616ca: The handling of funsafe-math-optimizations doesnt update the MathErrno (authored by zahiraam). Changed prior to commit:

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

2022-11-10 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D137107#3906766 , @rnk wrote: > In D137107#3905443 , @zahiraam > wrote: > >> extern int __declspec(dllimport) next(int n); >> int main () { >> extern int _declspec(dllimport)

[PATCH] D137578: Fix 'unsafe-math-optimizations' flag dependencies

2022-11-10 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D137578#3918314 , @michele.scandale wrote: > Looks good to me @michele.scandale Now that I opened up the patch to all, it needs another stamp from you. Would you mind reviewing it again please? I will wait for a day

[PATCH] D137578: Fix 'unsafe-math-optimizations' flag dependencies

2022-11-10 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3957 - if (Arg *A = Args.getLastArg(OPT_ffp_contract)) { -StringRef Val = A->getValue(); -if (Val == "fast") + if (auto *A = Args.getLastArg(OPT_ffp_contract,

[PATCH] D137578: Fix 'unsafe-math-optimizations' flag dependencies

2022-11-10 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 474515. zahiraam edited the summary of this revision. zahiraam added reviewers: andrew.w.kaylor, michele.scandale. zahiraam set the repository for this revision to rG LLVM Github Monorepo. zahiraam changed the visibility from "Custom Policy" to "All Users".

[PATCH] D137618: [Clang] Fix behavior of -ffp-model option when overriden

2022-11-09 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3034 RoundingFPMath = false; + FPExceptionBehavior = ""; // If fast-math is set then set the fp-contract mode to fast. FPExceptionBehavior should be set here

[PATCH] D137618: [Clang] Fix behavior of -ffp-model option when overriden

2022-11-08 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2844 FPContract = "fast"; +FPExceptionBehavior = ""; } else if (Val.equals("precise")) { I would expect the same thing should happen with -ffp-model=strict

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

2022-11-07 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D136786#3909043 , @zahiraam wrote: > In D136786#3907235 , > @michele.scandale wrote: > >> In D136786#3903646 , @zahiraam >> wrote: >> >>>

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

2022-11-07 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 473693. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137107/new/ https://reviews.llvm.org/D137107 Files: clang/lib/Sema/SemaDecl.cpp clang/test/CodeGenCXX/PR19955.cpp clang/test/CodeGenCXX/dllimport.cpp

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

2022-11-04 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D136786#3907235 , @michele.scandale wrote: > In D136786#3903646 , @zahiraam > wrote: > >> The changes in this patch look good to me. >> @michele.scandale please make sure not to

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

2022-11-03 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D137107#3906413 , @mstorsjo wrote: > In D137107#3905443 , @zahiraam > wrote: > >> and the same assembly (load __imp_?val) (is that why you made the comment >> above?). And the test

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

2022-11-03 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D137107#3897568 , @mstorsjo wrote: > In D137107#3897326 , @rnk wrote: > >> Unless I'm missing something, I think Clang's behavior here is preferable to >> MSVC's. MSVC produces code

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

2022-11-02 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. The changes in this patch look good to me. I talked to @andrew.w.kaylor offline: I was thinking that it might be necessary to make the two driver changes we talked about, before merging this patch. But if the tests pass then I think it's okay to implement the driver

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-11-01 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D123630#3899550 , @lenary wrote: > Reverse ping. This has been accepted, what is the status of landing this? As far as I can tell this has landed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

2022-10-31 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D137107#3897326 , @rnk wrote: > Unless I'm missing something, I think Clang's behavior here is preferable to > MSVC's. MSVC produces code which will not link. Clang turns the linker error > into a compiler error, which is

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

2022-10-31 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam created this revision. zahiraam added reviewers: majnemer, rnk. Herald added a project: All. zahiraam requested review of this revision. Herald added a project: clang. Microsoft allows the support of ‘constexpr’ with ‘__declspec(dllimport) starting from VS2017 15.u update 4. See

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

2022-10-31 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:181 + FPFeatures.allowFPContractAcrossStatement()); } Shouldn't then this also check for FPFeatures.getFPContractMode() ? Repository: rG LLVM Github Monorepo

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

2022-10-31 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D136786#3896708 , @michele.scandale wrote: > In D136786#3896341 , @zahiraam > wrote: > >>> I'm going to ignore fast-math right now, because I think the current >>> handling is

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

2022-10-31 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D136786#3893485 , @andrew.w.kaylor wrote: > In D136786#3892177 , @zahiraam > wrote: > I'm not following entirely, but -funsafe-math-optimizations is just a subset of

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

2022-10-28 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. >> I'm not following entirely, but -funsafe-math-optimizations is just a subset >> of -ffast-math, so checking only the properties that contribute to >> -funsafe-math-optimizations should be enough. >> I think it is way simpler to reason in these terms than

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

2022-10-27 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1865 FuncAttrs.addAttribute("approx-func-fp-math", "true"); -if ((LangOpts.FastMath || - (!LangOpts.FastMath && LangOpts.AllowFPReassoc && - LangOpts.AllowRecip &&

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-26 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1865-1869 +if ((LangOpts.FastMath || + !LangOpts.FastMath && LangOpts.AllowFPReassoc && LangOpts.AllowRecip && + !LangOpts.FiniteMathOnly && LangOpts.NoSignedZero && +

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-25 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1865-1869 +if ((LangOpts.FastMath || + !LangOpts.FastMath && LangOpts.AllowFPReassoc && LangOpts.AllowRecip && + !LangOpts.FiniteMathOnly && LangOpts.NoSignedZero && +

[PATCH] D109239: Add support for floating-option `-ffp-eval-method` and for new `pragma clang fp eval-method`

2022-10-24 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D109239#3878422 , @ArcsinX wrote: > It seems for x86_32 case (e.g. with -m32 in command line) there is a > difference between clang and GCC: > clang: `__FLT_EVAL_METHOD__` == 0 > GCC: `__FLT_EVAL_METHOD__` == 2 > > Example:

[PATCH] D136176: Implement support for option 'fexcess-precision'.

2022-10-21 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. @rjmccall Would you mind looking at this when you have a moment? Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136176/new/ https://reviews.llvm.org/D136176 ___

[PATCH] D136084: Fix LIT CodeGen/Func-attr.c

2022-10-17 Thread Zahira Ammarguellat via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd0a4741392a3: Fix LIT test func-attr.c added by https://reviews.llvm.org/D135097. (authored by zahiraam). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136084: Fix LIT CodeGen/Func-attr.c

2022-10-17 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 468236. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136084/new/ https://reviews.llvm.org/D136084 Files: clang/test/CodeGen/func-attr.c Index: clang/test/CodeGen/func-attr.c ===

[PATCH] D136084: Fix LIT CodeGen/Func-attr.c

2022-10-17 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/test/CodeGen/func-attr.c:1 -// RUN: %clang -c -ffast-math -emit-llvm -S -o - %s \ -// RUN: | FileCheck %s +// RUN: %clang -c -O2 -target x86_64 -ffast-math\ +// RUN: -emit-llvm -S -o - %s | FileCheck %s

[PATCH] D136084: Fix LIT CodeGen/Func-attr.c

2022-10-17 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. @saugustine A, @jrtc27. Thanks for your comments. Adding '-target x86_64' is restrictive, but not sure what option to use to express that I want target with zero as default address. @aaron.ballman what do you think? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D136084: Fix LIT CodeGen/Func-attr.c

2022-10-17 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam created this revision. zahiraam added reviewers: jrtc27, saugustine, aaron.ballman. Herald added a project: All. zahiraam requested review of this revision. Herald added a project: clang. See details in comment https://reviews.llvm.org/D135097#3859893 and

[PATCH] D135963: Fix build issue due to https://reviews.llvm.org/rG84a9ec2ff1ee97fd7e8ed988f5e7b197aab84a7b

2022-10-14 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D135963#3858638 , @aaron.ballman wrote: > LGTM -- btw, feel free to land fixes to buildbot breaks without code review. thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D135963: Fix build issue due to https://reviews.llvm.org/rG84a9ec2ff1ee97fd7e8ed988f5e7b197aab84a7b

2022-10-14 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam created this revision. zahiraam added a reviewer: aaron.ballman. Herald added a project: All. zahiraam requested review of this revision. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D135963 Files: clang/lib/CodeGen/CGCall.cpp Index:

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-14 Thread Zahira Ammarguellat via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. zahiraam marked an inline comment as done. Closed by commit rG84a9ec2ff1ee: Remove redundant option -menable-unsafe-fp-math. (authored by zahiraam). Repository: rG

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-14 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam marked an inline comment as done. zahiraam added inline comments. Comment at: clang/docs/ReleaseNotes.rst:410-412 +- The driver option ``-menable-unsafe-fp-math`` has been removed. Passing it, will +result in a hard error. To enable unsafe floating-point optimizations,

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-14 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 467749. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135097/new/ https://reviews.llvm.org/D135097 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Driver/Options.td clang/lib/CodeGen/CGCall.cpp clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-13 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/docs/ReleaseNotes.rst:410-412 +- The driver option ``-menable-unsafe-fp-math`` has been removed. Passing it, will +result in a hard error. To enable unsafe floating-point optimizations, the compiler +options

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-10 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 466474. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135097/new/ https://reviews.llvm.org/D135097 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Driver/Options.td clang/lib/CodeGen/CGCall.cpp clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-06 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D135097#3840706 , @aaron.ballman wrote: > I'm also okay with this direction. I took a look to see if people seemed to > be using this option in their build scripts (maybe we would need a louder > deprecation period), and

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-06 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 465790. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135097/new/ https://reviews.llvm.org/D135097 Files: clang/include/clang/Driver/Options.td clang/lib/CodeGen/CGCall.cpp clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-06 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 465768. zahiraam added reviewers: andrew.w.kaylor, aaron.ballman, rjmccall, efriedma, fhahn, dexonsmith. zahiraam changed the visibility from "Custom Policy" to "Public (No Login Required)". Herald added a subscriber: MaskRay. CHANGES SINCE LAST ACTION

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-09-15 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 460470. zahiraam marked 3 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/ https://reviews.llvm.org/D123630 Files: clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-09-15 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 460389. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/ https://reviews.llvm.org/D123630 Files: clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Clang.cpp clang/test/CodeGen/ffp-contract-option.c

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-09-14 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 460178. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/ https://reviews.llvm.org/D123630 Files: clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Clang.cpp clang/test/CodeGen/ffp-contract-option.c

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-09-13 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/docs/UsersManual.rst:1453-1455 + Note: ``DenormalFPMath`` and ``DenormalFP32Math`` are set by default to IEEE + (no flush) for ``-fno-fast-math``, ``-fno-unsafe-math-optimizations``, and + any setting of ``fp-model``. Clang

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-09-13 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 459893. zahiraam marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/ https://reviews.llvm.org/D123630 Files: clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-09-09 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 459028. zahiraam marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/ https://reviews.llvm.org/D123630 Files: clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-09-07 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. Thanks @aaron.ballman! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/ https://reviews.llvm.org/D123630 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-09-07 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 458506. zahiraam marked 4 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/ https://reviews.llvm.org/D123630 Files: clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-09-07 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. @fhahn @aaron.ballman would you mind taking time for a review for this patch? Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/ https://reviews.llvm.org/D123630 ___ cfe-commits mailing list

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-09-01 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 457358. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/ https://reviews.llvm.org/D123630 Files: clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Clang.cpp clang/test/CodeGen/ffp-contract-option.c

[PATCH] D123630: [WIP] Remove connection between 'ffast-math' and 'ffp-contract'.

2022-08-29 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 456337. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/ https://reviews.llvm.org/D123630 Files: clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Clang.cpp clang/test/CodeGen/ffp-contract-option.c

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-25 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a subscriber: bkramer. zahiraam added a comment. Thanks @bkramer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 ___ cfe-commits mailing list

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-24 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D113107#3747095 , @rjmccall wrote: > Thanks, LGTM Thanks for all the reviews. There is still more work to be done :) I will push it tomorrow as it's the end of day for me and I am afraid there will be some fails that I

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-23 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprComplex.cpp:615 + ComplexPairTy result = VisitMinus(E, promotionTy); + return result; +} rjmccall wrote: > This is not unpromoting if the original `PromotionType` is null. The idea for this

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-23 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D113107#3743318 , @rjmccall wrote: > Somehow we've taken a huge step back on unpromotion, and I'm worried you're > now doing the exact thing I didn't want us doing and forcing all the > downstream clients to handle the

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/test/CodeGen/X86/Float16-arithmetic.c:207 +// CHECK-NEXT:[[EXT:%.*]] = fpext half [[TMP0]] to float +// CHECK-NEXT:store float [[EXT]], ptr [[RETVAL]], align 2 +// CHECK-NEXT:[[TMP1:%.*]] = load half, ptr [[RETVAL]],

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam marked an inline comment as done. zahiraam added inline comments. Comment at: clang/test/CodeGen/X86/Float16-arithmetic.c:207 +// CHECK-NEXT:[[EXT:%.*]] = fpext half [[TMP0]] to float +// CHECK-NEXT:store float [[EXT]], ptr [[RETVAL]], align 2 +// CHECK-NEXT:

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D113107#3736325 , @rjmccall wrote: > In D113107#3736312 , @zahiraam > wrote: > >> This is a reduced test case from the codegen/complex-strictfp.c >> >> _Complex double g1, g2; >>

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-19 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. This is a reduced test case from the codegen/complex-strictfp.c _Complex double g1, g2; double D; void foo(void) { g1 = g1 + D; } The issue is that we are calling in VisitBinAssign the function EmitUnpromotion (since promotionTy is null). This creates 2

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-19 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam marked 3 inline comments as done. zahiraam added a comment. Comment at: clang/test/CodeGen/X86/Float16-complex.c:1184 +// X86-NEXT:store float [[NEG_I]], ptr [[RETVAL_IMAGP]], align 2 +// X86-NEXT:[[TMP0:%.*]] = load <2 x half>, ptr [[RETVAL]], align 2 +//

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-18 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprComplex.cpp:613 +result = EmitUnpromotion(promotionTy, E->getSubExpr()->getType(), result); + return result; +} rjmccall wrote: > You should unpromote only if we're not in a promoted

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-17 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprComplex.cpp:609 + ComplexPairTy Op; + PromotionType = getPromotionType(E->getSubExpr()->getType()); + if (!PromotionType.isNull()) rjmccall wrote: > This is overwriting the argument, so the

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-17 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. With this last patch, the behaviors of CodeGen/volatile-1.c and CodeGenCXX/volatile-1.cpp have changed ( change with __imag). I have uploaded the changes but still trying to figure out what exactly changed and why. Thanks. CHANGES SINCE LAST ACTION

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-17 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3140 + return CGF.Builder.CreateFPExt(result, ConvertType(E->getType())); + } + return result; rjmccall wrote: > zahiraam wrote: > > rjmccall wrote: > > > Please extract this

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-16 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. I think I introduced a bug when replacing the VisitUnaryMinus/Plus/Imag/Real with VisitMinus/Plus/Imag/Real. Now this simple test case is failing in the non-promotion path (with the +avx512fp16). _Float16 _Complex MinusOp_c_c(_Float16 c) { return -c; } error:

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-16 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam marked 2 inline comments as done. zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3140 + return CGF.Builder.CreateFPExt(result, ConvertType(E->getType())); + } + return result; rjmccall wrote: > Please extract this

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-12 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprComplex.cpp:950 +ComplexExprEmitter::EmitPromotedComplexOperand(const Expr *E, + QualType PromotionType) { + if (E->getType()->isAnyComplexType()) {

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-09 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. @pengfei , @rjmccall Thanks for the reviews. Sorry the updated patch took so long, I was on my sabbatical and returned back this week. I think I have responded to all the raised issues. Let me know if there is more to be done on this patch. Again thanks for the

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-09 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/test/CodeGen/X86/Float16-arithmetic.c:5-14 + // CHECK-LABEL: @add1 + // CHECK: [[A:%.*]] = alloca half + // CHECK-NEXT: [[B:%.*]] = alloca half + // CHECK: [[A_LOAD:%.*]] = load half, ptr [[A]] + // CHECK-NEXT: [[A_EXT:%.*]]

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-09 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 451261. zahiraam marked 7 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/include/clang/Basic/TargetInfo.h

[PATCH] D113107: Support of expression granularity for _Float16.

2022-07-01 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. Refactored the code as requested and fixed the compound operator promotion for scalar. The operators that are left to complete are compound operators for complex type and ternary operator for scalar and complex types. Then we need to add the option -fexcess-precision.

[PATCH] D113107: Support of expression granularity for _Float16.

2022-07-01 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 441702. zahiraam marked 7 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-29 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 441169. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Basic/TargetInfo.h clang/lib/Basic/Targets/X86.cpp

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-29 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. Fixed LIT tests. Fixed EmitPromoted for the Complex emitter (not 100% sure about it?). Added compound operator scalar emulation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-29 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 441167. zahiraam marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-29 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D113107#3615372 , @zahiraam wrote: > In D113107#3606106 , @rjmccall > wrote: > >> In D113107#3606094 , @zahiraam >> wrote: >> >>> In

[PATCH] D128814: [Clang][Preprocessor] Fix inconsistent `FLT_EVAL_METHOD` when compiling vs preprocessing

2022-06-29 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. Took the time to test the change on a few "crooked" tests I had used for the original patch. It works! LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128814/new/ https://reviews.llvm.org/D128814

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-28 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam marked an inline comment as done. zahiraam added a comment. In D113107#3606106 , @rjmccall wrote: > In D113107#3606094 , @zahiraam > wrote: > >> In D113107#3605797

[PATCH] D128571: [X86] Support `_Float16` on SSE2 and up

2022-06-27 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam accepted this revision. zahiraam added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128571/new/ https://reviews.llvm.org/D128571 ___ cfe-commits mailing list

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-23 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. For this test case: _Float16 add_half_cr(_Float16 a, _Float16 b) { return a > b ? a : b; } are we expecting the phi node to be cond.true: %2 = load float , ptr ... cond.false: %3 = load float, ptr ... %cond = phi float {{.*}} {{.*}} ? CHANGES SINCE

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-23 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D113107#3605797 , @rjmccall wrote: > I think on balance the right thing to do is probably to add an alternative to > `-fexcess-precision`, like `-fexcess-precision=none`. We can default to > `-fexcess-precision=standard`

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-23 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/docs/LanguageExtensions.rst:746 * SPIR -* X86 (Only available under feature AVX512-FP16) +* X86 (Enabled with feature SSE2 and up) rjmccall wrote: > Note that I'm not trying to propose this specific name for

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-23 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 439502. zahiraam marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-23 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 439385. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Basic/TargetInfo.h clang/lib/Basic/Targets/X86.cpp

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-23 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D113107#3604048 , @pengfei wrote: > @zahiraam, community requires to enable the `_Float16` support in FE, see > https://discourse.llvm.org/t/how-to-build-compiler-rt-for-new-x86-half-float-abi/63366 > Is there any blocking

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-23 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D113107#3603761 , @pengfei wrote: >> Supporting the lowering in the backend is sensible in order to support >> -fexcess-precision=16, because I agree that the most reasonable IR output in >> that configuration is to simply

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 439130. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Basic/TargetInfo.h clang/lib/Basic/Targets/X86.cpp

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 439128. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Basic/TargetInfo.h clang/lib/Basic/Targets/X86.cpp

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 439113. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Basic/TargetInfo.h clang/lib/Basic/Targets/X86.cpp

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 439110. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Basic/TargetInfo.h clang/lib/Basic/Targets/X86.cpp

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D113107#3601873 , @rjmccall wrote: > Supporting the lowering in the backend is sensible in order to support > `-fexcess-precision=16`, because I agree that the most reasonable IR output > in that configuration is to simply

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprComplex.cpp:896 + +ComplexPairTy ComplexExprEmitter::EmitPromoted(const Expr *E) { + if (auto *BinOp = dyn_cast(E->IgnoreParens())) { pengfei wrote: > zahiraam wrote: > > pengfei wrote: > > >

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 439020. zahiraam marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam marked 7 inline comments as done. zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprComplex.cpp:896 + +ComplexPairTy ComplexExprEmitter::EmitPromoted(const Expr *E) { + if (auto *BinOp = dyn_cast(E->IgnoreParens())) { pengfei wrote: >

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-17 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam marked 2 inline comments as done. zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprComplex.cpp:896 + +ComplexPairTy ComplexExprEmitter::EmitPromoted(const Expr *E) { + if (auto *BinOp = dyn_cast(E->IgnoreParens())) { rjmccall wrote:

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-15 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. @rjmccall I made some progress on the requests above. There are still a few things that need to be nailed down. The comma && ternary cases for EmitPromoted, I am not sure what needs to happen there? I am also not quite sure about the ComplexExpr’s generated IR . I

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-15 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 437191. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Basic/TargetInfo.h clang/lib/Basic/Targets/X86.cpp

[PATCH] D113107: Support of expression granularity for _Float16.

2022-06-03 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D113107#3554538 , @rjmccall wrote: > Hmm. I'm sorry, seeing the impact of this, I think it's not great to have > this `PromotionTy` field; it's too hard to get the invariants right. Let's do > this instead: > > 1. Add an

[PATCH] D113107: Support of expression granularity for _Float16.

2022-05-31 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment. In D113107#3520773 , @rjmccall wrote: > Okay, well, first off, we definitely shouldn't be repeating conditions like > `isX86()` all over the place. What you want to do is to have a general > predicate which answers whether we

<    1   2   3   4   5   6   >