Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

float instructions should only be lowered to NEON if precision constraints permit #16648

Open
tobiasgrosser opened this issue Jun 7, 2013 · 3 comments
Labels
backend:ARM bugzilla

Comments

@tobiasgrosser
Copy link
Contributor

@tobiasgrosser tobiasgrosser commented Jun 7, 2013

Bugzilla Link 16274
Version trunk
OS Linux
Attachments Test file attached
CC @rengolin,@stephenhines

Extended Description

On ARM it is difficult to generate optimal code that matches certain floating point precision requirements. The only way that currently exist is to explicitly alter the feature flags of the CPU that we target. This currently causes problems, such that it is e.g. not possible to take advantage of NEON for integer instructions while at the same time NEON is avoided for vector floating point operations.

The following test cases illustrate how I expect llc to behave:

; RUN: llc -march=arm -mattr=+vfp3,+neon,+neonfp < %s | FileCheck %s
; RUN: llc -march=arm -mattr=+vfp3,+neon,-neonfp -enable-flush-denormals-fp-math < %s | FileCheck %s -check-prefix=ALLOW-FLUSH

; fooP() performs a vector floating point multiplication with full precision
; requirement. Even if some piece of hardware supports NEON (modeled with
; -mattr=+neon), NEON should not be used to implement this function as NEON
; does not comply to the full precision requirements (NEON rounds denormals
; to zero)
;
; However, if the user specifies flushing denormals to zero is legal, we should
; obviously use NEON.
define <4 x float> @​fooP(<4 x float> %A, <4 x float> %B)
{
%C = fmul <4 x float> %A, %B
; CHECK: fooP
; CHECK: vmul.f32 s
; CHECK: vmul.f32 s
; CHECK: vmul.f32 s
; CHECK: vmul.f32 s

; CHECK-ALLOW-FLUSH: fooP
; CHECK-ALLOW-FLUSH: vmul.f32 q
ret <4 x float> %C
}

; fooR() performs a vector floating point multiplication with relaxed precision
; requirements. In case the precision loss introduced by neon is acceptable
; we should generate NEON instructions
;
; Relaxed precision requirements can be specified by using the fast-math flag
; fast or by introducing a specific allow-flush-denormals flag in LLVM-IR.
define <4 x float> @​fooR(<4 x float> %A, <4 x float> %B)
{
%C = fmul fast <4 x float> %A, %B

; We may decide to not implement this immediately as making decissions based on
; the floating point flags may require costum lowering of instructions.
; CHECK: fooR
; CHECK: vmul.f32 q

; CHECK-ALLOW-FLUSH: fooR
; CHECK-ALLOW-FLUSH: vmul.f32 q
ret <4 x float> %C
}

; fooS() perform a scalar floating point multiplication.
;
; On some ARM devices scalar floating point operations are faster when executed
; with NEON instructions. This is modeled by the '+neonfp' feature flag.
;
; Independently on which features the device supports (and which features are
; fast on a device) we should use NEON only if it matches the precision
; requirements of the target as provided by an -enable-flush-denormals-fp-math
; option. (The default for this option may differ on darwin and non-darwin
; systems).
define float @​fooS(float %A, float %B)
{
%C = fmul fast float %A, %B

; CHECK: fooS
; CHECK: vmul.f32 s
; CHECK: vmul.f32 s
; CHECK: vmul.f32 s
; CHECK: vmul.f32 s

; CHECK-ALLOW-FLUSH: fooS
; CHECK-ALLOW-FLUSH: vmul.f32 q
ret float %C
}

; bar() performs a vector integer multiplication. On an ARM NEON device, this
; code should always be execute as vector code, as floating point precision
; requirements do not apply.
define <4 x i32> @​bar(<4 x i32> %A, <4 x i32> %B)
{
%C = mul <4 x i32> %A, %B
; CHECK: bar
; CHECK: vmul.i32 q

; CHECK-ALLOW-FLUSH: bar
; CHECK-ALLOW-FLUSH: vmul.i32 q
ret <4 x i32> %C
}

@rengolin
Copy link
Member

@rengolin rengolin commented Jan 18, 2016

This may be a way to fix the fast-math issue in llvm/llvm-bugzilla-archive#16275 .

@rengolin
Copy link
Member

@rengolin rengolin commented Feb 12, 2016

After a long discussion on the list, this approach can be very problematic for NEON intrinsics (which require that NEON instructions be generated no matter what IEEE status or fast-math flags). Since IR doens't differentiate between code that has been produced by vectorizers or NEON intrinsics, we can't apply any serialization rule indiscriminately.

The only option left would be to have an extra command line option requesting IEEE compliance, and then it would be the user's responsibility to check the existence of NEON intrinsics, hand-crafted IR, etc.

This is also too big a hammer to fix #​16275, which already has its own fix.

All in all interesting, but too low on the priority list for me to work on it.

@tobiasgrosser
Copy link
Contributor Author

@tobiasgrosser tobiasgrosser commented Nov 26, 2021

mentioned in issue llvm/llvm-bugzilla-archive#16275

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:ARM bugzilla
Projects
None yet
Development

No branches or pull requests

2 participants