This is an archive of the discontinued LLVM Phabricator instance.

[Sparc] efficient pattern for UINT_TO_FP conversion
ClosedPublic

Authored by avl on Aug 18 2017, 7:32 AM.

Details

Summary
while investigating performance degradation of imagick benchmark
there were found inefficient pattern for UINT_TO_FP conversion.
That pattern causes RAW hazard in assembly code. Specifically,
uitofp IR operator results in poor assembler :

st          %i0, [%fp - 952]
ldd         [%fp - 952], %f0 

it stores 32-bit integer register into memory location and then 
loads 64-bit floating point data from that location. 
That is exactly RAW hazard case. To optimize that case it is 
possible to use SPISD::ITOF and SPISD::XTOF for conversion from 
integer to floating point data type and to use ISD::BITCAST to 
copy from integer register into floating point register.  
The fix is to write custom UINT_TO_FP pattern using SPISD::ITOF,
SPISD::XTOF, ISD::BITCAST.

Diff Detail

Event Timeline

avl created this revision.Aug 18 2017, 7:32 AM
avl added a comment.Aug 24 2017, 8:50 AM

Could someone review this fix, please ?

jyknight edited edge metadata.Oct 17 2017, 9:21 AM

Maybe a test for bitcast from int to float, without the uitofp, too?

lib/Target/Sparc/SparcISelLowering.cpp
1392

Move both these functions down below with the rest of the Lower* members (near where LowerUINT_TO_FP was previously) -- this section is for the calling convention lowering.

test/CodeGen/SPARC/uint_to_fp.ll
1

Tests for i64?
Tests for converting to float?

Do these overlap with some tests from "float.ll"? (If so, seems reasonable to pull those out here, too.)

test/CodeGen/SPARC/vis3.ll
1

Merge with the previous file?

avl updated this revision to Diff 119574.Oct 19 2017, 5:35 AM

updated a fix according to comments, specifically:

  • moved down LowerBITCAST, LowerUINT_TO_FP
  • moved tests into float.ll(it already has test for i64)
  • added test for bitcast from int to float(without uitofp)
  • added test for uitofp for float
avl added a comment.Nov 10 2017, 2:13 AM

Hi, Is the fix good now?

jyknight accepted this revision.Nov 10 2017, 8:24 AM

Seems reasonable.

This revision is now accepted and ready to land.Nov 10 2017, 8:24 AM
avl added a comment.EditedNov 11 2017, 1:44 PM

James, Fedor, Could you integrate this ? I do not have commit rights yet.

Thank you, Alexey.

This revision was automatically updated to reflect the committed changes.
fedor.sergeev edited edge metadata.Nov 20 2017, 2:37 PM

pushed.
Thanks for keeping up with it.