This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Transform a floating-point to fixed-point conversion to a VCVT_fix
ClosedPublic

Authored by samtebbs on Jun 23 2021, 10:00 AM.

Details

Summary

Much like fixed-point to floating-point conversion, the converse can also be transformed into a fixed-point VCVT. This patch transforms multiplications of floating point numbers by 2^n into a VCVT_fix. The exception is that a conversion with 1 fractional bit ends up being an FADD (FADD(x, x) emulates FMUL(x, 2)) rather than an FMUL so there is a special case for that. This patch also moves the code from https://reviews.llvm.org/D103903 into a separate function as fixed to float and float to fixed are very similar.

Diff Detail

Event Timeline

samtebbs created this revision.Jun 23 2021, 10:00 AM
samtebbs requested review of this revision.Jun 23 2021, 10:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 23 2021, 10:00 AM

Nice. I wasn't expecting this to share as much code, that's good to see.

llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp
3157

You can likely not pass dl, using SDLoc(N) again here.

3173–3174

auto -> SDValue

Should VecVal be different for the two cases? Otherwise below it uses the fmul->operand(0)->operand(0).

3176

Can this use getScalarSizeInBits?

3204

Maybe expand the comment to talk about both FixedToFloat and FloatToFixed

3254

auto -> EVT

3260

auto -> SDNode *

3310

The second N should be LHS?

llvm/test/CodeGen/Thumb2/mve-vcvt-float-to-fixed.ll
802

Does this need updating? and vcvt_u32_33 below.

986

These too.

samtebbs marked 7 inline comments as done.Jun 25 2021, 9:02 AM
samtebbs added inline comments.
llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp
3173–3174

Well spotted. I need to check if it's an [S|U]INT_TO_FP and assign it to operand 0 of itself if so.

3176

It certainly can.

llvm/test/CodeGen/Thumb2/mve-vcvt-float-to-fixed.ll
802

Yes, well spotted. Looks like I fell foul of copy and paste complacency.

samtebbs updated this revision to Diff 354519.Jun 25 2021, 9:07 AM
samtebbs marked 3 inline comments as done.

Stop passing dl, check if VecVal is an int to fp node, use getScalarSizeInBits() and fix copied tests.

dmgreen added inline comments.Jun 28 2021, 12:16 AM
llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp
3181

Should this still check for the same size of the bitcast on ImmNode?

3290

This can drop the brackets, from a single statement.

llvm/test/CodeGen/Thumb2/mve-vcvt-float-to-fixed.ll
312

Should this be 0x41E0000000000000 ?

324

And this 0x41F0000000000000

336

And this 0x4200000000000000 ?

1020

I'm not sure if these are interesting negative constants any more.

samtebbs marked 3 inline comments as done.Jun 29 2021, 6:05 AM
samtebbs added inline comments.
llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp
3181

It should :)

llvm/test/CodeGen/Thumb2/mve-vcvt-float-to-fixed.ll
312

Yes, you're right. I'd copied the same values from the fixed_to_float tests without remembering they were 1/2^n rather than 2^n.

samtebbs updated this revision to Diff 355200.Jun 29 2021, 6:13 AM
samtebbs marked 2 inline comments as done.

Check ImmNode's scalar bits, remove braces and fix constants in some tests.

samtebbs updated this revision to Diff 355220.Jun 29 2021, 6:46 AM
samtebbs marked 3 inline comments as done.

Comment about both fixed to float and float to fixed.

dmgreen accepted this revision.Jun 30 2021, 11:49 AM

LGTM

llvm/test/CodeGen/Thumb2/mve-vcvt-float-to-fixed.ll
794

0x41E0000000000000 :)

806

0x41F0000000000000

818

0x4200000000000000

This revision is now accepted and ready to land.Jun 30 2021, 11:49 AM
samtebbs marked 2 inline comments as done.EditedJul 1 2021, 6:58 AM

Thanks Dave. Thanks for spotting those other copy-paste mishaps :)

This revision was landed with ongoing or failed builds.Jul 1 2021, 7:12 AM
This revision was automatically updated to reflect the committed changes.