Page MenuHomePhabricator

[LegalizeTypes][VP] Add integer promotion support for vp.fptosi/vp.fptoui
ClosedPublic

Authored by Pretty-box on May 17 2022, 3:36 AM.

Diff Detail

Event Timeline

Pretty-box created this revision.May 17 2022, 3:36 AM
Pretty-box requested review of this revision.May 17 2022, 3:36 AM
craig.topper added inline comments.May 17 2022, 10:17 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
691

Merge this if into the else above to have else if

Pretty-box added inline comments.May 17 2022, 8:01 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
674

Does this code need to be added?Because ISD::VP_FPTOUI/ISD::VP_FPTOSI is considered illegal on RISCV(use custom model) . From the illegal FPTOUI test results, signed conversion instruction is used rather than unsigned conversion instruction. Is this result correct?@craig.topper

Pretty-box marked an inline comment as done.
craig.topper added inline comments.May 17 2022, 8:52 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
674

Does the same thing happen with non-vp for fixed vectors?

Pretty-box added inline comments.May 18 2022, 12:15 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
674

Yes, you can refer to the new added test cases,the fptoui tests using signed conversion instructions.

craig.topper added inline comments.May 18 2022, 4:39 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
674

I'd prefer to keep it for consistency. I don't have a good idea how to fix the ISD::FP_TO_UINT case right now.

686–689

Use curly braces for both elses to match the first if.

Pretty-box marked an inline comment as done.May 18 2022, 8:27 PM
Pretty-box added inline comments.
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
674

OK,Thanks for your advice.

craig.topper added inline comments.May 18 2022, 8:30 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
687

That's not what I meant.

I meant to be curly braces around the DAG node calls. LLVM coding standards say that if the if needs curly braces each of the elses do too.

See the 4th example here https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements "Use braces for the if block to keep it uniform with the else block."

Pretty-box added inline comments.May 18 2022, 8:49 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
687

Done. Thank you for your detailed explanation.

Pretty-box marked an inline comment as done.
This revision is now accepted and ready to land.May 29 2022, 6:57 PM
This revision was landed with ongoing or failed builds.May 29 2022, 8:06 PM
This revision was automatically updated to reflect the committed changes.