Fold (srl (mul (zext i32:$a to i64), i64:c), 32) -> (mulhu $a, $b) if c can truncate to i32 without loss.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
I'm adding some non-RVV reviewers in this field; that may help move this patch along.
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | ||
---|---|---|
8524 | I think the way HasConstant is interacting with checks below this could do with explanation (in the code). It's not clear to me why and how it interacts with logic later in this function. | |
8581–8582 | I might be narrowly-focussed here, but I'm not sure why we need a ternary here. Can't we capture RightOp.getOperand(0) in a variable and ensure it's correct in the the if (HasConstant) block? Would that simplify any of the other logic? |
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | ||
---|---|---|
8535 | Why not reuse LeftConstant/RightConstant? |
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | ||
---|---|---|
8518 | Do we need to worry about Constant LHS? Won't the mul be canonicalized and then the shift will be revisited? | |
8534 | Isn't ActiveBits the wrong thing to check for SIGN_EXTEND? | |
llvm/test/CodeGen/RISCV/rvv/vmulh-sdnode-rv32.ll | ||
69 | Was this test case supposed to match to mulh? |
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | ||
---|---|---|
8581–8582 | Done, thank you. |
Can the tests be pre-commited to trunk so we can see the effect of the patch on codegen?
llvm/test/CodeGen/RISCV/rvv/vmulh-sdnode-rv32.ll | ||
---|---|---|
1 | I have renamed this file to remove "-rv32" and added an rv64 command line. Do the same to the new file please. |
llvm/test/CodeGen/RISCV/rvv/vmulh-sdnode-rv32.ll | ||
---|---|---|
1 | Done, thanks a lot. |
llvm/test/CodeGen/RISCV/rvv/vmulhu-sdnode.ll | ||
---|---|---|
3 ↗ | (On Diff #383244) | One of these RUN lines should be -mtriple=riscv64 |
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | ||
---|---|---|
8522 | Can you re-write this to if (!IsSignExt && !ZeroExt) |
Please can you rebase on top of D110675 - the whole point of the precommit is to show the codegen diff
Do we need to worry about Constant LHS? Won't the mul be canonicalized and then the shift will be revisited?