julian-seward1 opened PR #2168 from fix-no-shift-by-imm
to main
:
The logic for generation of shifts-by-immediate was not quite right. The result was that even
shifts by an amount known at compile time were being done by moving the shift immediate into %cl
and then doing a variable shift by %cl. The effect is worse than it sounds, because all of
those shift constants are small and often used in multiple places, so they were GVN'd up and
often ended up at the entry block of the function. Hence these were connected to the use points
by long live ranges which got spilled. So all in all, most of the win here comes from avoiding
spilling.The problem was caused by this line, in the
Opcode::Ishl | Opcode::Ushr ..
case:let (count, rhs) = if let Some(cst) = ctx.get_constant(inputs[1].insn) {
inputs[]
appears to refer to this CLIF instruction's inputs, and bizarrelyinputs[].insn
all
refer to the instruction (the shift) itself. Hencectx.get_constant(inputs[1].insn)
asks
"does this shift instruction produce a constant" to which the answer is always "no", so the
shift-by-unknown amount code is always generated. The fix here is to change that expression tolet (count, rhs) = if let Some(cst) = ctx.get_input(insn, 1).constant {
get_input
's result conveniently includes aconstant
field of typeOption<u64>
, so we just
use that instead.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
julian-seward1 requested bnjbvr for a review on PR #2168.
bnjbvr submitted PR Review.
bnjbvr merged PR #2168.
Last updated: Dec 23 2024 at 12:05 UTC