Discussion:
RFA: fix mode confusion in caller-save.c:replace_reg_with_saved_mem
Joern Rennecke
2014-10-06 08:51:56 UTC
Permalink
Investigating an ICE while trying to compile libgcc2.c:__udivmoddi4 for
a new avr variant with different register set/allocation order, I found
replace_reg_with_saved_mem falling over its own nonsense. The instruction:

(debug_insn 97 96 98 2 (var_location:SI __x2 (mult:SI (lshiftrt:SI
(reg/v:SI 18 r18 [orig:58 q0 ] [58])
(const_int 16 [0x10]))
(reg/v:SI 61 [ __vl ])))
../../gcc/gcc/testsuite/gcc.target/avr/tiny-caller-save.c:67 -1
(nil))

would be transformed into:

(debug_insn 97 96 98 2 (var_location:SI __x2 (mult:SI (lshiftrt:SI (concatn:SI [
(reg:SI 18 r18)
(reg:SI 19 r19)
(mem/c:QI (plus:HI (reg/f:HI 28 r28)
(const_int 43 [0x2b])) [6 S1 A8])
(mem/c:QI (plus:HI (reg/f:HI 28 r28)
(const_int 44 [0x2c])) [6 S1 A8])
])
(const_int 16 [0x10]))
(reg/v:SI 61 [ __vl ])))
../../gcc/gcc/testsuite/gcc.target/avr/tiny-caller-save.c:67 -1

Note that r18 and r19 inside the concatn are supposed to be single hard
registers, but as the word size of this processor is 8 bit, SImode
extends actually
over four hard registers. save_mode is SImode because four registers can be
saved/restored at once.

The attached patch fixes the failure by using word_mode if smode would cover
multiple hard registers.
bootstrapped & regtested on i686-pc-linux-gnu.

OK to apply?
Jeff Law
2014-10-06 18:58:35 UTC
Permalink
Post by Joern Rennecke
Investigating an ICE while trying to compile libgcc2.c:__udivmoddi4 for
a new avr variant with different register set/allocation order, I found
(debug_insn 97 96 98 2 (var_location:SI __x2 (mult:SI (lshiftrt:SI
(reg/v:SI 18 r18 [orig:58 q0 ] [58])
(const_int 16 [0x10]))
(reg/v:SI 61 [ __vl ])))
../../gcc/gcc/testsuite/gcc.target/avr/tiny-caller-save.c:67 -1
(nil))
(debug_insn 97 96 98 2 (var_location:SI __x2 (mult:SI (lshiftrt:SI (concatn:SI [
(reg:SI 18 r18)
(reg:SI 19 r19)
(mem/c:QI (plus:HI (reg/f:HI 28 r28)
(const_int 43 [0x2b])) [6 S1 A8])
(mem/c:QI (plus:HI (reg/f:HI 28 r28)
(const_int 44 [0x2c])) [6 S1 A8])
])
(const_int 16 [0x10]))
(reg/v:SI 61 [ __vl ])))
../../gcc/gcc/testsuite/gcc.target/avr/tiny-caller-save.c:67 -1
Note that r18 and r19 inside the concatn are supposed to be single hard
registers, but as the word size of this processor is 8 bit, SImode
extends actually
over four hard registers. save_mode is SImode because four registers can be
saved/restored at once.
The attached patch fixes the failure by using word_mode if smode would cover
multiple hard registers.
bootstrapped & regtested on i686-pc-linux-gnu.
OK to apply?
What makes word_mode special here? ie, why is special casing for
word_mode the right thing to do?

jeff
Joern Rennecke
2014-10-07 02:57:21 UTC
Permalink
What makes word_mode special here? ie, why is special casing for word_mode
the right thing to do?
The patch does not special-case word mode. The if condition tests if
smode would
cover multiple hard registers.
If that would be the case, smode is replaced with word_mode.
Jeff Law
2014-10-07 17:38:11 UTC
Permalink
Post by Joern Rennecke
What makes word_mode special here? ie, why is special casing for word_mode
the right thing to do?
The patch does not special-case word mode. The if condition tests if
smode would
cover multiple hard registers.
If that would be the case, smode is replaced with word_mode.
SO I'll ask another way. Why do you want to change smode to word_mode?

Jeff
Joern Rennecke
2014-10-07 21:56:20 UTC
Permalink
Post by Jeff Law
Post by Joern Rennecke
What makes word_mode special here? ie, why is special casing for word_mode
the right thing to do?
The patch does not special-case word mode. The if condition tests if
smode would
cover multiple hard registers.
If that would be the case, smode is replaced with word_mode.
SO I'll ask another way. Why do you want to change smode to word_mode?
Because SImode covers four hard registers, wheras the intention is to
have a single
one.

(concatn:SI [
(reg:SI 18 r18)
(reg:SI 19 r19)
(mem/c:QI (plus:HI (reg/f:HI 28 r28)
(const_int 43 [0x2b])) [6 S1 A8])
(mem/c:QI (plus:HI (reg/f:HI 28 r28)
(const_int 44 [0x2c])) [6 S1 A8])
])

(see original post) is invalid RTL, and thuis the cause of the later ICE.
Loading...