This is an archive of the discontinued LLVM Phabricator instance.

[Legalizer] Promote result type in expanding FP_TO_XINT
ClosedPublic

Authored by qiucf on Dec 2 2020, 3:40 AM.

Details

Summary

This patch promotes result integer type of FP_TO_XINT in expanding. Crash in conversion from ppc_fp128 to i1 will be fixed.

Diff Detail

Event Timeline

qiucf created this revision.Dec 2 2020, 3:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2020, 3:40 AM
qiucf requested review of this revision.Dec 2 2020, 3:40 AM
qiucf updated this revision to Diff 308928.

Fix type bug

qiucf updated this revision to Diff 308955.Dec 2 2020, 6:55 AM

Split the common methods stuff into another revision. Rebase this upon D92481.

craig.topper added inline comments.Dec 2 2020, 1:14 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
916

If the type isn't legal, it would have already been handled by LegalizeIntegerTypes.cpp since results are checked for legality first.

949–951

This won't execute findConversionLibcall in a release build. assert is a macro, nothing passed to it will be evaluated if asserts aren't enabled.

qiucf updated this revision to Diff 309163.Dec 2 2020, 11:37 PM
qiucf marked 2 inline comments as done.

Update comments and call findConversionLibcall outside assert.

qiucf added a comment.Dec 17 2020, 9:35 PM

Gentle ping

nemanjai added inline comments.Dec 28 2020, 8:15 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
916

I think Conversion is to vague here. Perhaps findFPToIntLibcall()?

qiucf updated this revision to Diff 313920.Dec 28 2020, 6:45 PM
qiucf marked an inline comment as done.

Rename findConversionLibcall.

steven.zhang added inline comments.Jan 5 2021, 10:06 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
1911

Also add the assertion for NVT to make it more safe ? assert(NVT.isSimple() ...)

qiucf updated this revision to Diff 315695.Jan 10 2021, 7:47 PM
qiucf marked an inline comment as done.

Assert target type is simple

steven.zhang accepted this revision.Jan 10 2021, 7:50 PM

LGTM, but please hold on for days to see if @craig.topper or @nemanjai have more comments.

This revision is now accepted and ready to land.Jan 10 2021, 7:50 PM
This revision was automatically updated to reflect the committed changes.