This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] DAG Combine vcpop and vfirst with VL=0 to li imm
ClosedPublic

Authored by Chenbing.Zheng on Feb 21 2022, 10:57 PM.

Details

Summary

vcpop and vfirst are still useful when VL=0.
vcpop equivalents to li 0 and vfirst equivalents to li -1,
since no mask elements are active.

Diff Detail

Event Timeline

Chenbing.Zheng requested review of this revision.Feb 21 2022, 10:57 PM
craig.topper added a comment.EditedFeb 21 2022, 11:04 PM

This should be done as a DAGCombine not isel. You want the constant folding to enable other optimizations.

Even better would be instcombine so we can fold away branches. SelectionDAG can’t remove a branch even if the condition becomes constant.

Chenbing.Zheng retitled this revision from [RISCV] Expend vcpop and vfirst with VL=0 to li imm to [RISCV] DAG Combine vcpop and vfirst with VL=0 to li imm.

move to DAGCombine

craig.topper added inline comments.Feb 22 2022, 10:09 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
8492

You can use isNullConstant(VL)

8497

Use N->getValueType(0), not the type of the VL.

8503

There's no reason to make an ISD::ADD. You can just return DAG.getConstant

fix according to review comments

Chenbing.Zheng marked 3 inline comments as done.Feb 22 2022, 10:38 PM
craig.topper added inline comments.Feb 23 2022, 4:30 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
8500

No need for Imm. You can return DAG.getConstant(-1, DL, VT);

8501

No need for Imm. You can return DAG.getConstant(0, DL, VT);

Chenbing.Zheng marked 2 inline comments as done.Feb 23 2022, 5:53 PM

I pre-commit tests in D120300, could you accept it if this patch is OK ?

This revision is now accepted and ready to land.Feb 24 2022, 9:00 AM
This revision was landed with ongoing or failed builds.Feb 24 2022, 10:52 PM
This revision was automatically updated to reflect the committed changes.