https://github.com/jcranmer-intel closed
https://github.com/llvm/llvm-project/pull/80475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/jcranmer-intel updated
https://github.com/llvm/llvm-project/pull/80475
>From 971cc613e994a308f939f68247257b65e04c74fa Mon Sep 17 00:00:00 2001
From: Joshua Cranmer
Date: Fri, 2 Feb 2024 10:35:29 -0800
Subject: [PATCH 1/5] Disable FTZ/DAZ when compiling shared libraries by
de
andykaylor wrote:
> One final nit before you submit, though: please update the PR description and
> the release notes to also mention that x86 no longer modifies
> `-fdenormal-fp-math` based on `-ffast-math`.
The Clang User's Manual makes some contradictory statements about this. In one
place
https://github.com/MaskRay approved this pull request.
https://github.com/llvm/llvm-project/pull/80475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/MaskRay edited
https://github.com/llvm/llvm-project/pull/80475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1307,9 +1307,14 @@ void ToolChain::AddCCKextLibArgs(const ArgList &Args,
bool ToolChain::isFastMathRuntimeAvailable(const ArgList &Args,
std::string &Path) const {
+ // Don't implicitly link in mode-changing libraries in a shared
https://github.com/jcranmer-intel updated
https://github.com/llvm/llvm-project/pull/80475
>From 971cc613e994a308f939f68247257b65e04c74fa Mon Sep 17 00:00:00 2001
From: Joshua Cranmer
Date: Fri, 2 Feb 2024 10:35:29 -0800
Subject: [PATCH 1/4] Disable FTZ/DAZ when compiling shared libraries by
de
@@ -842,25 +842,6 @@ void Linux::addProfileRTLibs(const llvm::opt::ArgList
&Args,
ToolChain::addProfileRTLibs(Args, CmdArgs);
}
-llvm::DenormalMode
-Linux::getDefaultDenormalModeForType(const llvm::opt::ArgList &DriverArgs,
- const JobAct
https://github.com/jcranmer-intel edited
https://github.com/llvm/llvm-project/pull/80475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -842,25 +842,6 @@ void Linux::addProfileRTLibs(const llvm::opt::ArgList
&Args,
ToolChain::addProfileRTLibs(Args, CmdArgs);
}
-llvm::DenormalMode
-Linux::getDefaultDenormalModeForType(const llvm::opt::ArgList &DriverArgs,
- const JobAct
https://github.com/andykaylor approved this pull request.
https://github.com/llvm/llvm-project/pull/80475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -842,25 +842,6 @@ void Linux::addProfileRTLibs(const llvm::opt::ArgList
&Args,
ToolChain::addProfileRTLibs(Args, CmdArgs);
}
-llvm::DenormalMode
-Linux::getDefaultDenormalModeForType(const llvm::opt::ArgList &DriverArgs,
- const JobAct
https://github.com/jyknight approved this pull request.
One final nit before you submit, though: please update the PR description and
the release notes to also mention that x86 no longer modifies
`-fdenormal-fp-math` based on `-ffast-math`.
https://github.com/llvm/llvm-project/pull/80475
_
https://github.com/arsenm approved this pull request.
https://github.com/llvm/llvm-project/pull/80475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -842,25 +842,6 @@ void Linux::addProfileRTLibs(const llvm::opt::ArgList
&Args,
ToolChain::addProfileRTLibs(Args, CmdArgs);
}
-llvm::DenormalMode
-Linux::getDefaultDenormalModeForType(const llvm::opt::ArgList &DriverArgs,
- const JobAct
@@ -842,25 +842,6 @@ void Linux::addProfileRTLibs(const llvm::opt::ArgList
&Args,
ToolChain::addProfileRTLibs(Args, CmdArgs);
}
-llvm::DenormalMode
-Linux::getDefaultDenormalModeForType(const llvm::opt::ArgList &DriverArgs,
- const JobAct
jcranmer-intel wrote:
Ping for review
https://github.com/llvm/llvm-project/pull/80475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/jcranmer-intel updated
https://github.com/llvm/llvm-project/pull/80475
>From 971cc613e994a308f939f68247257b65e04c74fa Mon Sep 17 00:00:00 2001
From: Joshua Cranmer
Date: Fri, 2 Feb 2024 10:35:29 -0800
Subject: [PATCH 1/3] Disable FTZ/DAZ when compiling shared libraries by
de
arsenm wrote:
> But the shared library stuff isn't an issue for AMDGPU, right?
No, we don't support shared library linking yet
https://github.com/llvm/llvm-project/pull/80475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.o
jcranmer-intel wrote:
> It matters more for AMDGPU, where we need to care because some instructions
> just don't respect denormals. We legalize some operations differently
> depending on the mode
But the shared library stuff isn't an issue for AMDGPU, right?
https://github.com/llvm/llvm-proje
arsenm wrote:
> So overall, the practical effect of the `denormal-fp-math` attribute being
> set incorrectly doesn't appear to matter.
It matters more for AMDGPU, where we need to care because some instructions
just don't respect denormals. We legalize some operations differently depending
on
@@ -1569,6 +1569,40 @@
// RUN:--gcc-toolchain="" \
// RUN:--sysroot=%S/Inputs/basic_linux_tree 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-NOCRTFASTMATH %s
+// Don't link crtfastmath.o with -shared
+// RUN: %clang --target=x86_64-unknown-linux -no-pie -###
jcranmer-intel wrote:
I did a thorough investigation into how the
`denormal-fp-math=preserve-sign,preserve-sign` attribute affects the resulting
IR for all of the SPEC benchmarks (which actually do run into subnormals), and
the basic summary I found is that the only differences I found were th
https://github.com/jcranmer-intel updated
https://github.com/llvm/llvm-project/pull/80475
>From fc0507f013f556cc7c49a38f22d14578311f1f42 Mon Sep 17 00:00:00 2001
From: Joshua Cranmer
Date: Fri, 2 Feb 2024 10:35:29 -0800
Subject: [PATCH 1/3] Disable FTZ/DAZ when compiling shared libraries by
de
@@ -117,6 +117,11 @@ C23 Feature Support
Non-comprehensive list of changes in this release
-
+* Code compiled with ``-shared`` and ``-ffast-math`` will no longer enable
andykaylor wrote:
```suggestion
* Code com
@@ -117,6 +117,11 @@ C23 Feature Support
Non-comprehensive list of changes in this release
-
+* Code compiled with ``-shared`` and ``-ffast-math`` will no longer enable
+ flush-to-zero floating-point mode by default. This decisi
@@ -1569,6 +1569,40 @@
// RUN:--gcc-toolchain="" \
// RUN:--sysroot=%S/Inputs/basic_linux_tree 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-NOCRTFASTMATH %s
+// Don't link crtfastmath.o with -shared
+// RUN: %clang --target=x86_64-unknown-linux -no-pie -###
andykaylor wrote:
> I don't think it's unreasonable to switch the logic of `-mdaz-ftz` from
> linking a file with a global initializer that sets the flags to making it
> emit the entry-point call to such a function instead, it still largely
> follows the same logic to me. And having a command
jcranmer-intel wrote:
> > > I'd like to see this change land, but with the "-mdaz-ftz" option removed
> > > (because I don't think it's useful). That would fix the critical problem
> > > of fast-math infecting shared libraries with the ftz setting, and we
> > > could straighten out the other p
andykaylor wrote:
I just created https://github.com/llvm/llvm-project/issues/81204 to describe
the linking versus compiling problem.
https://github.com/llvm/llvm-project/pull/80475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.
andykaylor wrote:
> > I don't see why the current denormal-fp-math setting behavior is a blocking
> > issue for this change
>
> Because this PR as-is causes us to start parsing the "-shared" flag for
> compilation actions in order to determine which denormal-fp-math setting to
> use. Users sh
andykaylor wrote:
> > I'd like to see this change land, but with the "-mdaz-ftz" option removed
> > (because I don't think it's useful). That would fix the critical problem of
> > fast-math infecting shared libraries with the ftz setting, and we could
> > straighten out the other problems, whi
jcranmer-intel wrote:
> I'd like to see this change land, but with the "-mdaz-ftz" option removed
> (because I don't think it's useful). That would fix the critical problem of
> fast-math infecting shared libraries with the ftz setting, and we could
> straighten out the other problems, which a
jyknight wrote:
> I don't see why the current denormal-fp-math setting behavior is a blocking
> issue for this change
Because this PR as-is causes us to start parsing the "-shared" flag for
compilation actions in order to determine which denormal-fp-math setting to
use. Users should not pass
andykaylor wrote:
> I'm suggesting that we modify Clang so that `-ffast-math` _doesn't affect_
> `denormal-fp-math`, by (as I mentioned before) removing the overload
> [Linux::getDefaultDenormalModeForType](https://github.com/llvm/llvm-project/blob/d4c5acac99e83ffa12d2d720c9e502a181cbd7ea/clang
jyknight wrote:
I'm proposing a simple change we can make _now_, in order to unblock this PR
which at least gets rid of crtfastmath for shared libraries! I don't think this
needs to be the end of the story; additional, more large-scale changes can be
made afterwards...
> Are youe suggesting t
andykaylor wrote:
> Just to be clear: I'm not proposing entirely eliminating the "link against
> crtfastmath.o" behavior, when linking a binary with `-ffast-math` (though,
> separately from _this_ review, that may be worth considering!). I only meant
> we should stop attempting to infer anythi
jyknight wrote:
> > > > So, alternatively...we could just go with the simplest solution, and
> > > > use "ieee" as the default even under -ffast-math.
> >
> >
> > +1. **There hasn't been a performance reason to use FTZ/DAZ since ~2011.**
> > Maybe there's still a power benefit? But in that ca
arsenm wrote:
> Do you only set the register for kernel entries?
Yes, it's the pre-initialized state. Non kernels can't be arbitrarily invoked
from the host
> Is the attribute ignored for other functions?
No, it's an informative attribute about that the mode is. The compiler isn't
trying t
andykaylor wrote:
> > > So, alternatively...we could just go with the simplest solution, and use
> > > "ieee" as the default even under -ffast-math.
>
> +1. **There hasn't been a performance reason to use FTZ/DAZ since ~2011.**
> Maybe there's still a power benefit? But in that case you could
andykaylor wrote:
> > I may have mentioned a few times that I don't like function attributes
> > controlling fast-math behaviors.
>
> It doesn't control it, it's informative. You just get undefined behavior if
> you end up calling mismatched mode functions.
What I meant to say was that I don'
arsenm wrote:
> > So, alternatively...we could just go with the simplest solution, and use
> > "ieee" as the default even under -ffast-math.
>
+1. There hasn't been a performance reason to use FTZ/DAZ since ~2011. Maybe
there's still a power benefit? But in that case you could still explicitl
jyknight wrote:
> So, alternatively...we could just go with the simplest solution, and use
> "ieee" as the default even under -ffast-math.
This is feeling like the best option to me, at this point. Easily
implementable, and doesn't seem to make things significantly worse than they
are today,
arsenm wrote:
> I may have mentioned a few times that I don't like function attributes
> controlling fast-math behaviors.
It doesn't control it, it's informative. You just get undefined behavior if you
end up calling mismatched mode functions.
It does control it in the AMDGPU entry point func
andykaylor wrote:
> > (Sidenote: "dynamic" isn't even
> > [documented](https://clang.llvm.org/docs/UsersManual.html#cmdoption-fdenormal-fp-math)).
>
> It's not a selectable enum of the Clang `-fdenormal-fp-math` flag, but it is
> one for the LLVM function attribute `denormal-fp-math`.
This is
andykaylor wrote:
I don't know anything about how non-x86 targets implement DAZ/FTZ, but for
x86-based targets, I think trying to make any assumptions about the setting is
bound to be wrong. In theory, it's part of the floating-point environment and
shouldn't be modified during execution unles
jcranmer-intel wrote:
> (Sidenote: "dynamic" isn't even
> [documented](https://clang.llvm.org/docs/UsersManual.html#cmdoption-fdenormal-fp-math)).
It's not a selectable enum of the Clang `-fdenormal-fp-math` flag, but it is
one for the LLVM function attribute `denormal-fp-math`.
https://githu
jyknight wrote:
> You'll get different results depending on whether the input is implicitly
> flushed in fcmp vs. not in the is.fpclass
This sounds intuitively like the sort of semantics-breaking optimization which
is expected from `-ffast-math`. If the only issues are things like getting a
s
jcranmer-intel wrote:
> I think there is a bit of a problematic interaction with
> getDenormalModeForType
> [here](https://github.com/llvm/llvm-project/blob/7a94acb2da5b20d12f13f3c5f4eb0f3f46e78e73/clang/lib/Driver/ToolChains/Linux.cpp#L838C8-L838C37).
> "-shared" is (should be) a flag used on
arsenm wrote:
> * Which value allows generating the "fastest" math code -- disregarding
> correctness? I'd assume that "dynamic" is least optimizable, "ieee" in the
> middle, and "preserve-sign" is likely to generate the "fastest" code?
This depends on the target and operations. For some funct
jyknight wrote:
> It is not always safe to run preserve-sign code under IEEE settings
I can see that this is used in a bunch of optimization/constant-folding passes,
but I don't have a feel for what the actual impact is:
1. Which value allows generating the "fastest" math code -- disregarding
arsenm wrote:
> I wonder if, instead, we should just have `-ffast-math` always downgrade
> `-fdenormal-fp-math=ieee` to `-fdenormal-fp-math=preserve-sign`, under the
> rationale of "you asked for fast math, and preserve-sign mode might let the
> compiler generate faster code"?
This could also
jyknight wrote:
I think there is a bit of a problematic interaction with getDenormalModeForType
[here](https://github.com/llvm/llvm-project/blob/7a94acb2da5b20d12f13f3c5f4eb0f3f46e78e73/clang/lib/Driver/ToolChains/Linux.cpp#L838C8-L838C37).
"-shared" is (should be) a flag used only for linking,
@@ -1569,6 +1569,40 @@
// RUN:--gcc-toolchain="" \
// RUN:--sysroot=%S/Inputs/basic_linux_tree 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-NOCRTFASTMATH %s
+// Don't link crtfastmath.o with -shared
+// RUN: %clang --target=x86_64-unknown-linux -no-pie -###
@@ -2554,6 +2554,11 @@ defm protect_parens : BoolFOption<"protect-parens",
"floating-point expressions are evaluated">,
NegFlag>;
+defm daz_ftz : SimpleMFlag<"daz-ftz",
+ "Globally set", "Do not globally set",
+ " the denormals-are-zero (DAZ) and flush-to-zero (F
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Joshua Cranmer (jcranmer-intel)
Changes
This fixes https://github.com/llvm/llvm-project/issues/57589, and aligns Clang
with the behavior of current versions of gcc. There is a new option, -mdaz-ftz,
to control the linking of the file that
https://github.com/jcranmer-intel created
https://github.com/llvm/llvm-project/pull/80475
This fixes https://github.com/llvm/llvm-project/issues/57589, and aligns Clang
with the behavior of current versions of gcc. There is a new option, -mdaz-ftz,
to control the linking of the file that sets
57 matches
Mail list logo
