Segher Boessenkool schrieb:
On Thu, Dec 22, 2016 at 04:18:34PM +0100, Georg-Johann Lay wrote:
If you don't have instruction scheduling subregs of mem are allowed (and
are counted as registers). Combine asks recog, and it think this is
fine.
Why does reload use r31 here? Why does it think that is valid for
HImode?
I can't tell you, all I'm seeing bunch of new FAILs after r243578.
The avr port does not define CANNOT_CHANGE_MODE_CLASS. It probably
should.
Thanks for that pointer, I'll try it as soon as I can.
i.e. the paradoxical subreg could be resolved somehow and R25 is
uninitialized. It this the purpose of the combine change to come
up with uninitialized regs because it is known that just the
initialized parts are used by the code?
The purpose of the combine change is to write widening extracts in a
more general form, so that backends for processors that can do such
more general things do not have to write hundreds (literally) extra
patterns for all the cases that could be written as zero_extract.
One problem is that not all expressions are canonicalized and combine
might come up with many different kinds of representations for the
same action. One common example is inserting one bit. This might
be represented as (set (zero_extract)) or as set with masking and
shifting around or as (set (if_then_else)), with different
representations if the sign bit is involved or if the source
bit position is the same or lower or higher than the destination's
bit position.
In a private back end I had the same problem that I didn't want to
support dozens of combine patterns and added a new hook that allows
the back end to canonicalize expressions synthesized by combine.
This runs right before recog(_for_combine) and can replace single_set
by equivalent ones. This makes also simplifies porting the back end
because just one place in combine has to be touches and not hundreds
of places in combine.c. Moreover, different targets might come up
with different, conflicting preferences so that a one-fits-all
solution in combine.c doesn't always exist anyway.
(insn 98 97 58 9 (set (reg:QI 31 r31)
(mem:QI (reg:HI 30 r30) [1 *operands_17(D)+0 S1 A8]))
"pr26833.c":11 56 {movqi_insn}
(nil))
(jump_insn 58 98 59 9 (set (pc)
(if_then_else (eq (and:HI (reg:HI 31 r31)
(const_int 1 [0x1]))
(const_int 0 [0]))
(label_ref 70)
(pc))) "pr26833.c":11 415 {*sbrx_and_branchhi}
(int_list:REG_BR_PROB 375 (nil))
-> 70)
insn 98 is valid but overrides parts of HI:30 set by insn 97.
As it appears, the QI memory is loaded to R31 which is valid, and
then reload drops in that register as replacement for the paradoxical
subreg and comes up with HI:31 in insn 58.
Actually a zero-extend would be needed, does it?
The AND clears the top bits already.
OK, I didn't know that the register allocator analyses that parts of
a value (high part in this case) are effectively unused and skips
the extension. Cool feature.
Johann
Please correct me if I'm wrong, but I see no bugs in the combined
expressions.
Algebraically it looks correct, but I am unsure if reload is supposed
to handle such situations.
It is valid RTL. reload should handle it.
And I must admit I never saw "paradoxical"
extractions before; I would expected an extension in that case, not an
extraction.
An extension _of an extraction_. Widening extractions are valid (and I
didn't see them until a few weeks ago either, heh -- that's why Dominik
needed this patch ;-) )
Segher
