On Mon, Nov 16, 2020 at 2:45 PM Philipp Tomsich <philipp.toms...@vrull.eu>
wrote:
> This is an de-optimization only, if applied without patch 1 from the
> series: the change to VRP ensures that the backend will never see a shift
> wider than the immediate field.
> The problem is that if a negative shift-amount makes it to the backend,
> unindented code may be generated (as a shift-amount, in my reading, should
> always be interpreted as unsigned).
>
I doubt that you are catching every possible case. What kind of testing
have you done to prove this? The code you are removing does nothing
harmful, but removing it may de-optimize code. So it should not be removed
without a very good reason and a lot of testing to make sure there is no
regression.
Shift counts do not have to be unsigned at the assembly language level.
Consider this testcase
int sub (int i, int j) { return i << (32 - j); }
Compiling with rv32 -O2 gives me
li a5,32
sub a5,a5,a1
sll a0,a0,a5
which is 3 instructions. But since we know that shift counts are
truncated, we should be compiling this as
neg a1,a1
sll a0,a0,a1
which gives the exact same result with 2 instructions. This is on my todo
list. This deliberately uses a negative shift count, but this is well
defined in the RISC-V ISA, so this poses no problems.
There are other related things we can do here to improve code generation
for shifts. We should not be limiting optimization at the assembly level
by what the C standard says.
You also might want to look at SHIFT_COUNT_TRUNCATED and
TARGET_SHIFT_TRUNCATION_MASK. We already have infrastructure to handle
out-of-range shifts as the hardware dictates. Your changes conflict with
this. I think we should define TARGET_SHIFT_TRUNCATION_MASK for RISC-V.
That is another item on my todo list.
Jim
