Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

[RISCV] Improve codegen for i32 udiv/urem by constant on RV64.

Authored by craig.topper on Nov 12 2021, 1:41 PM.



The division by constant optimization often produces constants that
are uimm32, but not simm32. These constants require 3 or 4 instructions
to materialize without Zba.

Since these instructions are often used by a multiply with a LHS
that needs to be zero extended with an AND, we can switch the MUL
to a MULHU by shifting both inputs left by 32. Once we shift the
constant left, the upper 32 bits no longer need to be 0 so constant
materialization is free to use LUI+ADDIW. This reduces the constant
materialization from 4 instructions to 3 in some cases while also
reducing the zero extend of the LHS from 2 shifts to 1.

Diff Detail

Event Timeline

craig.topper created this revision.Nov 12 2021, 1:41 PM
craig.topper requested review of this revision.Nov 12 2021, 1:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 12 2021, 1:41 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
mcberg2021 added inline comments.Nov 12 2021, 2:09 PM

Looks like the TODO is done making that part of the comment no longer needed

craig.topper added inline comments.Nov 12 2021, 2:37 PM

I think when I wrote the TODO I meant to add something like these patterns

(i64 (mul (and GPR:$rs1, 0xffffffff), (assertzexti32 GPR:$rs2)))


(i64 (mul (assertzexti32 GPR:$rs1), (and GPR:$rs2, 0xffffffff)))

Where assertzexti32 covers AssertZExt and anything that computeKnownBits can prove is has only 32 active bits. In that case we'd insert a SLLI on the assertexti32 side without reducing any instructions for that operand. If that operand is the critical path it would make it worse. If the AND has an additional use it would be an increase in code size.

SelectionDAG doesn't normally check the critical path like that, so maybe I should drop the TODO based on the fact that we couldn't check that.

This revision is now accepted and ready to land.Nov 12 2021, 2:46 PM
This revision was landed with ongoing or failed builds.Nov 12 2021, 3:01 PM
This revision was automatically updated to reflect the committed changes.