This patch supports VP_MERGE, VP_SELECT, SELECT, SELECT_CC for fp16 vectors when only have Zvfhmin.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 14485 | I'm not sure what "failed promotion" means | |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 14485 | I rewrite this comment, it should be a bit more clear. | |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 14486 | What prevents this for constants that aren't 0? | |
| llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp | ||
|---|---|---|
| 5509 ↗ | (On Diff #554211) | This seems like it might be a RISC-V specific issue and not something we should do in target specific code. What code is reversing the promotion? That code should be checking isOperationLegal |
| llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp | ||
|---|---|---|
| 5509 ↗ | (On Diff #554211) | If you end up doing this check, either here on in RISCV specific code, you can simplify to if (isa<ConstantSDNode>(Scalar)) |
| llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp | ||
|---|---|---|
| 5509 ↗ | (On Diff #554211) | Thanks for your advice, I use implement it in RISCV target specific logic now. |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 14485 | How does this approach compare to making the constant folder of getNode not do the fold when only have zvfhmin? | |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 14485 | I have no idea about how to make the constant folder of getNode not do the fold when only have zvfhmin if we just want to change the RISCV target specific code. Do you have some advice? | |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 14485 | Here is one idea (using pseudo code): Target::isConstantFoldLegal(InformationAboutFold) { return true; }
RISCVTarget::isConstantFoldLegal(InformationAboutFold I) override {
if (isConstantFoldFromF32ToF16(I) && onlyHaveZvfhmin())
return false;
return true;
}
ConstantFolder::fold(MachineInstr NotFoldedYet, InformationAboutFold I) {
if (Target.isConstantFoldLegal(I))
return NotFoldedYet;
return doFold(NotFoldedYet);
}The comment in the code points out that the current solution is a work around to avoid a problem that would have been introduced by the constant folder. If we solve it in the constant folder then we avoid needing to do a work around and allow the lowering to happen at the normal time. That being said, I'm not confident that there is anything wrong with lowering early since I am not an expert in this area. I wonder if you have any thoughts (or @craig.topper when he is back from vacation). | |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 14485 | I think I'd rather see this implemented as a custom lowering of SPLAT_VECTOR rather than a combine. That likely means we need to manually implement the promotion of SPLAT_VECTOR for the non-constant case in the same lowering since we can't use Promote for setOperationAction, but that doesn't seem like a blocker. | |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 14485 | Done, I switched to the custom lowering. | |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 6014 | The break; is unreachable due to the return above it. | |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 6014 | Fixed. | |
The break; is unreachable due to the return above it.