This is an archive of the discontinued LLVM Phabricator instance.

[TargetLowering] Improve vXi64 UITOFP vXf64 support (P38226)
ClosedPublic

Authored by RKSimon on Oct 24 2018, 8:38 AM.

Details

Summary

As suggested on D52965, this patch moves the i64 to f64 UINT_TO_FP expansion code from LegalizeDAG into TargetLowering, generalizes it to work for vectors and makes it available to LegalizeVectorOps as well.

Not only does this help perform X86 lowering as a true vectorization instead of (partially vectorized) scalar conversions, it avoids the HADDPD op from the scalar code which can be slow on most targets.

My only minor concern is that AVX512F does have the vcvtusi2sdq scalar operation but we don't use it as (a) for some reason we don't set it as legal, and (b) it seems to only help for the v2f64 case - otherwise the unrolling cost will certainly be too high. My feeling is that we should leave it to the vectorizers - and if it generates the vector UINT_TO_FP we should use it.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Oct 24 2018, 8:38 AM
craig.topper accepted this revision.Oct 24 2018, 10:33 PM

LGTM

Not sure I understood the AVX512F comment about vcvtusi2sdq not being legal. Isn't it allowed with AVX512F in X86TargetLowering::LowerUINT_TO_FP? It can't be explicitly marked Legal with setOperationAction because that interface only mentions the input type and we still have to checkout the output type.bold text

include/llvm/CodeGen/TargetLowering.h
3659 ↗(On Diff #170905)

Might want to include the types in the function name?

This revision is now accepted and ready to land.Oct 24 2018, 10:33 PM

LGTM

Not sure I understood the AVX512F comment about vcvtusi2sdq not being legal. Isn't it allowed with AVX512F in X86TargetLowering::LowerUINT_TO_FP? It can't be explicitly marked Legal with setOperationAction because that interface only mentions the input type and we still have to checkout the output type.

Thanks - you're right, unless we add more annoying _Int patterns I don't see a way around it.

include/llvm/CodeGen/TargetLowering.h
3659 ↗(On Diff #170905)

I'm intending to move other ui2fp expansions over as well, so I'll keep these general.

This revision was automatically updated to reflect the committed changes.