This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.
On 21.12.2016 15:42, Dominik Vogt wrote:
On Wed, Dec 21, 2016 at 01:58:18PM +0100, Georg-Johann Lay wrote:
On 12.12.2016 17:54, Segher Boessenkool wrote:
On Mon, Dec 12, 2016 at 05:46:02PM +0100, Dominik Vogt wrote:
Patch with these changes and a fix because of not handling
VOIDmode attached. Bootstrapped and regression tested on s390 and
s390x.
Okay for trunk.
When did you see VOIDmode, btw? It wasn't on a const_int I hope?
Segher
* combine.c (change_zero_ext): Handle mode expanding zero_extracts.
This was committes as r243578 and triggered (amongst other similar
test suite ICE failures):
$ avr-gcc
/gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/compile/pr26833.c
-S -O1 -mmcu=avr4 -S -v
/gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/compile/pr26833.c:
In function 'yasm_lc3b__parse_insn':
/gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/compile/pr26833.c:19:1:
error: insn does not satisfy its constraints:
}
^
(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))) "/gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/compile/pr26833.c":11
415 {*sbrx_and_branchhi}
(int_list:REG_BR_PROB 375 (nil))
-> 70)
Does that mean that, on avr, this is valid:
(zero_extract:HI (reg:QI 31) ...)
but this is not:
(subreg:HI (reg:QI 31))
?
What exactly do you mean by valid? The second is just a paradoxical
subreg, dunno if a BE can disallow paradoxical subregs.
As the insn has a HImode register_operand, the subreg might be valid
before reload, the extract is certainly not a register_operand.
I don't know how exactly reload handles paradoxical subregs...
presumably reloading into a valid HImode register and applying
signedness as appropriate for the extension.
Re-quoting the insn...
which cannot be correct because avr.md::*sbrx_and_branchhi reads:
(define_insn "*sbrx_and_branch<mode>"
[(set (pc)
(if_then_else
(match_operator 0 "eqne_operator"
[(and:QISI
(match_operand:QISI 1 "register_operand" "r")
(match_operand:QISI 2 "single_one_operand" "n"))
(const_int 0)])
(label_ref (match_operand 3 "" ""))
(pc)))]
"" { ... } ...)
...there is nothing involving (explicit) extracts or paradoxical regs.
FYI, the dump prior to .combine (.init-regs) reads as follows,
no extracts or paradoxical subregs around...
(note 54 53 55 7 [bb 7] NOTE_INSN_BASIC_BLOCK)
(insn 55 54 56 7 (set (reg:HI 79 [ *operands_17(D) ])
(mem:HI (reg/v/f:HI 75 [ operands ]) [1 *operands_17(D)+0 S2
A8])) "pr26833.c":11 68 {*movhi}
(nil))
(insn 56 55 57 7 (parallel [
(set (reg:HI 78)
(and:HI (reg:HI 79 [ *operands_17(D) ])
(const_int 1 [0x1])))
(clobber (scratch:QI))
]) "pr26833.c":11 251 {andhi3}
(expr_list:REG_DEAD (reg:HI 79 [ *operands_17(D) ])
(nil)))
(insn 57 56 58 7 (parallel [
(set (cc0)
(compare (reg:HI 78)
(const_int 0 [0])))
(clobber (scratch:QI))
]) "pr26833.c":11 398 {cmphi3}
(expr_list:REG_DEAD (reg:HI 78)
(nil)))
(jump_insn 58 57 59 7 (set (pc)
(if_then_else (eq (cc0)
(const_int 0 [0]))
(label_ref 70)
(pc))) "pr26833.c":11 418 {branch}
(int_list:REG_BR_PROB 375 (nil))
-> 70)
If combine wants to throw away unneeded portions of regs 78 / 79
and apply some kind of demotion, then it could use plain QImode,
at least in the present case where a similar QI version of the
insn is available.
Johann
Hence we have a memory operand (subreg of mem)) where only a
register is allowed. Reg alloc then reloads the mem:QI into R31,
but R31 is the
last hard reg, i.e. R31 cannot hold HImode.
Johann
Ciao
Dominik ^_^ ^_^