♟ rampitec

LGTM except comment typo.

Remove now unused NumOps variable. Otherwise LGTM.

I think this is right about round and extend.

[AMDGPU] Use AssumptionCacheTracker in the divrem32 expansion

Fix llvm::ComputeNumSignBits with some operations and llvm.assume

So given the discussion you seem to be missing DAG.isKnownNeverNaN(Op) condition.

Should we care about sNaNs with FP exceptions disabled?

Yes. This has to work. The OpenCL conformance tests check for this

I think we need a diff against master.

In D49561#1169653, @arsenm wrote:

In D49561#1169015, @rampitec wrote:

In D49561#1168791, @arsenm wrote:

In D49561#1168643, @rampitec wrote:

I would tend to say isKnownNeverSNan here basically tells not that it cannot be sNaN, but rather that we do not care even if it is. At least we should not care if FP exceptions are off.

Except that's not true since there can be sNaNs can be loaded, so even if exceptions aren't handled they need to be dealt with

Do you mean we shall quiet snans with an fmin/fmax even if exception handling is off? My understanding was different. In fact I thought we can completely ignore them with say fast math.

Yes. My understanding is the exception handling aspect is different from the existence of sNaNs, and also orthogonal to enabling fast math. If exception handling is enabled, operations that can produce sNaNs will, but there can still be sNaNs loaded from memory, constants etc. I think if the function is marked no-nans, we can maybe assume this won't happen? The fast math flags aren't sufficient there because arbitrary operations like a load or function argument don't have an associated flag.

With ieee_mode enabled (which is currently the default) v_min_f32 et. al. a qNaN is returned if either input is an sNaN. If ieee_mode is off, it returns the non-NaN operand as-if it were a qNaN. I think for what we actually want, ieee_mode is harmful since it requires the library implementation of OpenCL's fmin to insert canonicalizes to quiet the inputs. Since LLVM doesn't properly handle sNaNs anywhere, I think enabling this is a bit pointless. However, whether it's on or not, I think we need to to get sNaN behavior correct at least for this one operation in order to be able to optimize out redundant canonicalizes.

First, I think this is wrong diff attached, that is not what is in the trunk on the left side of the diff now.

At the very least it is knownNeverNan, then it cannot be a signaling nan as well.

In D49561#1168791, @arsenm wrote:

In D49561#1168643, @rampitec wrote:

I would tend to say isKnownNeverSNan here basically tells not that it cannot be sNaN, but rather that we do not care even if it is. At least we should not care if FP exceptions are off.

Except that's not true since there can be sNaNs can be loaded, so even if exceptions aren't handled they need to be dealt with

I would tend to say isKnownNeverSNan here basically tells not that it cannot be sNaN, but rather that we do not care even if it is. At least we should not care if FP exceptions are off.

In D49146#1157765, @FarhanaAleen wrote:

As far as I understand it should be also legal with -mattr=-fp32-denormals,-fp64-fp16-denormals. I.e. when both 32 and 16 denorms are not supported. Right? Not that is really helps in the real world.
Otherwise it shall be legal if either UnsafeAlgebra or AllowContract flag is set on both FMA nodes.

Having the FMA node already grantees that either UnsafeAlgebra is set or AllowContract flag set is on the FAdd/FMUL nodes. We don't need to check them again during the FMA combine, right?

In D49146#1157632, @b-sumner wrote:

This operation only rounds a single time, and unfortunately always flushes f32 denorms. Thus this transformation should only be done when unsafe math is requested.

Does fdot2 perform rounding of intermediates?
Basically you start with two FMAs: FMA - Perform a * b + c with no intermediate rounding step. So the expression you are converting is quite fancy in terms of rounding:

Can you please add tests for <3 x i64> and <3 x double>?

[AMDGPU] Enable LICM in the BE pipeline

Moved LICM post SROA.

[AMDGPU] Early expansion of 32 bit udiv/urem

[AMDGPU] Overload llvm.amdgcn.fmad.ftz to support f16

In D48573#1144712, @arsenm wrote:

Actually according to the selection code, f16 mad does not support denormals the same, so the intrinsic should work with f16 if that is correct

[AMDGPU] Convert rcp to rcp_iflag

Changed hi/lo split method as suggested.

[AMDGPU] Add llvm.amdgcn.fmad.ftz intrinsic

In D48586#1143943, @arsenm wrote:

Won't doing this break the case where both the div and rem are used, so the full expansion will be used twice?

Added tests with source modifiers.

rampitec (Stanislav Mekhanoshin)
User

Projects

User Details

Recent Activity
View All

Today

Thu, Aug 16

Wed, Aug 15

Tue, Aug 14

Mon, Aug 13

Sun, Aug 12

Thu, Aug 9

Wed, Aug 8

Tue, Aug 7

Mon, Aug 6

Thu, Aug 2

Tue, Jul 31

Mon, Jul 30

Thu, Jul 26

Wed, Jul 25

Tue, Jul 24

Jul 20 2018

Jul 19 2018

Jul 18 2018

Jul 17 2018

Jul 16 2018

Jul 13 2018

Jul 12 2018

Jul 10 2018

Jul 9 2018

Jun 29 2018

Jun 28 2018

Jun 27 2018

Jun 26 2018

rampitec (Stanislav Mekhanoshin)User