This patch adjusts the legality check for riscv to use cpop/cpopw since isOperationLegal(ISD::CTPOP, MVT::i32) returns false on rv64gc_zbb.
Clang vs gcc: https://godbolt.org/z/rc3s4hjPh
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
18223 | The vector part of this isn't tested. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
18223 | I think if we check that the operation is legal then we don’t need to check for zbb/zvbb, since it’s only legal with those extensions |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
18223 | I am not familiar with the riscv vector crypto extension. I just do the same check as https://github.com/llvm/llvm-project/blob/3c4d2221de0e9e7b259a308aab7e72e3f2f926f1/llvm/lib/Target/RISCV/RISCVISelLowering.cpp#L764. It is worth noting that isOperationLegal(ISD::CTPOP, MVT::i32) returns false on rv64gc_zbb. We will miss some optimizations if we only check the legality of ctpop. |
llvm/test/CodeGen/RISCV/rvv/ctpop-sdnode.ll | ||
---|---|---|
860 | These tests already pass. |
llvm/test/CodeGen/RISCV/rvv/ctpop-sdnode.ll | ||
---|---|---|
860 |
Should I remove these useless tests? We always emit vcpop for the vector part. |
llvm/test/CodeGen/RISCV/rvv/ctpop-sdnode.ll | ||
---|---|---|
860 | You can keep them. Just be clear in the description that they already pass. We do need additional tests for fixed vector without "vscale x". I bet they don't pass without this patch. |
llvm/test/CodeGen/RISCV/rv64zbb.ll | ||
---|---|---|
700 | I meant with V and the Zvbb extensions enabled so we have vector cpop instructions. |
llvm/test/CodeGen/RISCV/rvv/ctpop-sdnode.ll | ||
---|---|---|
1512 | These fixed-vector tests should be moved into fixed-vectors-ctpop.ll. |
The vector part of this isn't tested.