Page MenuHomePhabricator

[MLIR/GPUToNVVM] lowering arith.maxf/minf to __nv_fmax/__nv_fmin
AbandonedPublic

Authored by qingyunqu on Aug 3 2022, 8:21 PM.

Details

Summary

Lowering arith.maxf/minf to __nv_fmax/__nv_fmin in GPUToNVVM Conversion.
Also remove Linalg pass declaration which is not cleaned.

Diff Detail

Event Timeline

qingyunqu created this revision.Aug 3 2022, 8:21 PM
qingyunqu requested review of this revision.Aug 3 2022, 8:21 PM
qingyunqu set the repository for this revision to rG LLVM Github Monorepo.Aug 3 2022, 8:23 PM
qingyunqu retitled this revision from [MLIR/GPUToNVVM] lowering aritch.maxf/minf to __nv_fmaxf/__nv_fminf to [MLIR/GPUToNVVM] lowering arith.maxf/minf to __nv_fmax/__nv_fmin.
qingyunqu edited the summary of this revision. (Show Details)
qingyunqu updated this revision to Diff 449859.Aug 3 2022, 8:33 PM

Also remove Linalg pass declaration which is not cleaned.

qingyunqu edited the summary of this revision. (Show Details)Aug 3 2022, 8:34 PM
ThomasRaoux added inline comments.Aug 3 2022, 9:06 PM
mlir/include/mlir/Dialect/Linalg/Passes.h
34 ↗(On Diff #449859)

Is this on purpose?

ThomasRaoux added inline comments.Aug 3 2022, 9:11 PM
mlir/include/mlir/Dialect/Linalg/Passes.h
34 ↗(On Diff #449859)

never mind I see your comment and this is dead.

mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
257

The semantic of arith::MaxFOp and __nv_fmax_f are different for Nan values:

arith::MaxFOp:

Returns the maximum of the two arguments, treating -0.0 as less than +0.0.
If one of the arguments is NaN, then the result is also NaN.

__nv_fmax_f:

If one argument is a NaN and the other is legitimate numeric value, the numeric
value is chosen.
qingyunqu added inline comments.Aug 3 2022, 9:16 PM
mlir/include/mlir/Dialect/Linalg/Passes.h
34 ↗(On Diff #449859)

this pass seems to be removed at https://reviews.llvm.org/D124145

qingyunqu added inline comments.Aug 3 2022, 9:37 PM
mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
257

This seems to be a problem. BTW, does other math function have the same problem? like math.sin(NaN) and __nv_sin(NaN).

ThomasRaoux added inline comments.Aug 3 2022, 9:41 PM
mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
257

Yes I don't think we can have this lowering with the current semantic of arith::MaxFOp as far as I know this problem is only for fmax/fmin as the semantic of TF and other ML framework tends to be different than what hw natively support. For other arithmetic operations the semantic is more obvious.

mravishankar added inline comments.Aug 3 2022, 10:19 PM
mlir/include/mlir/Dialect/Linalg/Passes.h
34 ↗(On Diff #449859)

Thanks!

qingyunqu added inline comments.Aug 3 2022, 11:25 PM
mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
257

Thanks, should I close this differential?

ThomasRaoux added inline comments.Aug 3 2022, 11:44 PM
mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
257

Yes unfortunately I don’t think there is another solution right now.

qingyunqu abandoned this revision.Aug 3 2022, 11:47 PM