This is an archive of the discontinued LLVM Phabricator instance.

[X86] Optimize fdiv with reciprocal instructions for half type
ClosedPublic

Authored by pengfei on Sep 27 2021, 8:18 AM.

Diff Detail

Event Timeline

pengfei created this revision.Sep 27 2021, 8:18 AM
pengfei requested review of this revision.Sep 27 2021, 8:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2021, 8:18 AM
pengfei updated this revision to Diff 377263.Oct 5 2021, 8:36 AM

Fix scalar problem.

pengfei retitled this revision from [X86][WIP] Optimize fdiv with reciprocal instructions for half type to [X86] Optimize fdiv with reciprocal instructions for half type.Oct 5 2021, 8:40 AM
RKSimon added inline comments.Oct 5 2021, 8:46 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
23182

This is a SCALAR_TO_VECTOR node

23233

SCALAR_TO_VECTOR

pengfei updated this revision to Diff 377276.Oct 5 2021, 9:12 AM
pengfei marked 2 inline comments as done.

Thanks Simon. I had seen the discussion about INSERT_VECTOR_ELT and SCALAR_TO_VECTOR, but forgot the conclusion. Do you remember the reason why we prefer the latter?

craig.topper added inline comments.Oct 5 2021, 9:23 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
23177

Should we keep the user RefinementStep if it isn't ReciprocalEstimate::Unspecified?

23228

Same question as above.

pengfei updated this revision to Diff 377415.Oct 5 2021, 6:58 PM
pengfei marked an inline comment as done.

Keep the user RefinementStep when ReciprocalEstimate::Unspecified.

llvm/lib/Target/X86/X86ISelLowering.cpp
23177

Yes, we should respect user RefinementStep here. Thanks Craig.

craig.topper added inline comments.Oct 6 2021, 9:27 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
23227

You need to know the type is legal. v64f16 will get through this, but type legalization won't be able to split a v64f16 X86ISD::RCP14 node.

pengfei updated this revision to Diff 377863.Oct 7 2021, 8:18 AM

Address review comment.

llvm/lib/Target/X86/X86ISelLowering.cpp
23227

Yes. Thanks Craig!

This revision is now accepted and ready to land.Oct 7 2021, 10:53 AM