Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2017-01-04 Thread Dominik Vogt
On Fri, Dec 23, 2016 at 05:54:01PM +0100, Georg-Johann Lay wrote:
> 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.

I'm working on patches to get more sensible simplify results in
some of these cases, like extracting the sign bit.  Not really
canonical, but rather that dealing with every odd combination it's
better to suppress them.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-23 Thread Segher Boessenkool
On Fri, Dec 23, 2016 at 05:54:01PM +0100, Georg-Johann Lay wrote:
> >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.

Yeah.

> 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.

The problem with this is that you create new RTL for every insn fed
to recog (so, a lot more insns then there are in your program).  That
isn't very nice (combine tries very hard to create as little garbage
as it can).

> 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.

I don't understand what you mean here, what hundreds of places in
combine?  What is "porting", here?

> 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.

Yeah.

> 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.

It was a paradoxical subreg before, the RA just changes that to hard
regs?


Segher


Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-23 Thread Georg-Johann Lay

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




Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-22 Thread Segher Boessenkool
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.

> >>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.

> >>(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.

> >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


Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-22 Thread Georg-Johann Lay

On 22.12.2016 15:27, Dominik Vogt wrote:

On Thu, Dec 22, 2016 at 12:00:37PM +0100, Georg-Johann Lay wrote:

On 21.12.2016 18:46, Segher Boessenkool wrote:

On Wed, Dec 21, 2016 at 01:58:18PM +0100, Georg-Johann Lay wrote:

$ 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)




Combine comes up with the following insn:
(jump_insn 58 57 59 7 (set (pc)
   (if_then_else (eq (and:HI (subreg:HI (mem:QI (reg/v/f:HI 75 [
operands ]) [1 *operands_17(D)+0 S1 A8]) 0)
   (const_int 1 [0x1]))
   (const_int 0 [0]))
   (label_ref 70)
   (pc)))
"/home/georg/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)

which cannot be correct because avr.md::*sbrx_and_branchhi reads:

(define_insn "*sbrx_and_branch"
 [(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)))]
 "" { ... } ...)

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.


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.
For a simpler test case like the following

unsigned u;
int volatile v;

void btest1 (void)
{
if (u & 1)
v = 0;
v = 0;
}

the combine pattern is also used but the register pressure is smaller.
After quite some spills, reload then comes up with

(insn 18 7 8 2 (set (reg:QI 24 r24)
(mem/c:QI (symbol_ref:HI ("u")  )
[1 u+0 S1 A8])) "testbit.c":6 56 {movqi_insn}
 (nil))
(jump_insn 8 18 9 2 (set (pc)
(if_then_else (eq (and:HI (reg:HI 24 r24)
(const_int 1 [0x1]))
(const_int 0 [0]))
(label_ref 11)
(pc))) "testbit.c":6 415 {*sbrx_and_branchhi}
 (int_list:REG_BR_PROB 4600 (nil))
 -> 11)

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?


If a

  (zero_extract (foo) (const_int) (const_int)

does not match in combine, it calls change_zero_ext to replace it
with

  (and (lshiftrt (foo) (const_int)) (const_int)

(The zero_extract expression is generated by combine for insn
55+56->57.)  Prior to the patch, this was done only if the
zero_extract has the same mode as "foo".  With the patch, it also
deals with mode expanding zero_extracts, e.g.

  (zero_extract:DI (foo:SI) (const_int) (const_int))

->

  (and:DI (subreg:DI (lshiftrt:SI (const_int) 0) (const_int))

This is what combine has done:

Trying 55, 56 -> 57:
...
Failed to match this instruction:
(set (reg:HI 78)
(zero_extract:HI (mem:QI (reg/v/f:HI 75 [ operands ]) [1
*operands_17(D)+0 S1 A8])
(const_int 1 [0x1])
(const_int 0 [0])))
Successfully matched this instruction:
(set (reg:HI 78)
(and:HI (subreg:HI (mem:QI (reg/v/f:HI 75 [ operands ]) [1
*operands_17(D)+0 S1 A8]) 0)
(const_int 1 [0x1])))
Successfully matched this instruction:
(set (cc0)
(compare (reg:HI 78)
(const_int 0 [0])))
allowing combination of insns 55, 56 and 57

Later it combinded insn 57 -> 58 to the expression that has
triggered the bad register allocation.  This looks all correct to
me.


Even if subregs are allowed, paradoxical subregs look really odd here.

The ICE'ing test case, reload comes up with

(insn 97 57 98 9 (set (reg:HI 30 r30)
(reg/v/f:HI 20 r20 [orig:75 operands ] [75])) "pr26833.c":11
68 {*movhi}
 (nil))
(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]))

Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-22 Thread Dominik Vogt
On Thu, Dec 22, 2016 at 12:00:37PM +0100, Georg-Johann Lay wrote:
> On 21.12.2016 18:46, Segher Boessenkool wrote:
> >On Wed, Dec 21, 2016 at 01:58:18PM +0100, Georg-Johann Lay wrote:
> >>$ 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)
> >
> >
> >>Combine comes up with the following insn:
> >>(jump_insn 58 57 59 7 (set (pc)
> >>(if_then_else (eq (and:HI (subreg:HI (mem:QI (reg/v/f:HI 75 [
> >>operands ]) [1 *operands_17(D)+0 S1 A8]) 0)
> >>(const_int 1 [0x1]))
> >>(const_int 0 [0]))
> >>(label_ref 70)
> >>(pc)))
> >>"/home/georg/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)
> >>
> >>which cannot be correct because avr.md::*sbrx_and_branchhi reads:
> >>
> >>(define_insn "*sbrx_and_branch"
> >>  [(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)))]
> >>  "" { ... } ...)
> >>
> >>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.
> >
> >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.
> For a simpler test case like the following
> 
> unsigned u;
> int volatile v;
> 
> void btest1 (void)
> {
> if (u & 1)
> v = 0;
> v = 0;
> }
> 
> the combine pattern is also used but the register pressure is smaller.
> After quite some spills, reload then comes up with
> 
> (insn 18 7 8 2 (set (reg:QI 24 r24)
> (mem/c:QI (symbol_ref:HI ("u")  )
> [1 u+0 S1 A8])) "testbit.c":6 56 {movqi_insn}
>  (nil))
> (jump_insn 8 18 9 2 (set (pc)
> (if_then_else (eq (and:HI (reg:HI 24 r24)
> (const_int 1 [0x1]))
> (const_int 0 [0]))
> (label_ref 11)
> (pc))) "testbit.c":6 415 {*sbrx_and_branchhi}
>  (int_list:REG_BR_PROB 4600 (nil))
>  -> 11)
> 
> 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?

If a

  (zero_extract (foo) (const_int) (const_int)

does not match in combine, it calls change_zero_ext to replace it
with

  (and (lshiftrt (foo) (const_int)) (const_int)

(The zero_extract expression is generated by combine for insn
55+56->57.)  Prior to the patch, this was done only if the
zero_extract has the same mode as "foo".  With the patch, it also
deals with mode expanding zero_extracts, e.g.

  (zero_extract:DI (foo:SI) (const_int) (const_int))

->

  (and:DI (subreg:DI (lshiftrt:SI (const_int) 0) (const_int))

This is what combine has done:

Trying 55, 56 -> 57:
...
Failed to match this instruction:
(set (reg:HI 78)
(zero_extract:HI (mem:QI (reg/v/f:HI 75 [ operands ]) [1
*operands_17(D)+0 S1 A8])
(const_int 1 [0x1])
(const_int 0 [0])))
Successfully matched this instruction:
(set (reg:HI 78)
(and:HI (subreg:HI (mem:QI (reg/v/f:HI 75 [ operands ]) [1
*operands_17(D)+0 S1 A8]) 0)
(const_int 1 [0x1])))
Successfully matched this instruction:
(set (cc0)
(compare (reg:HI 78)
(const_int 0 [0])))
allowing combination of insns 55, 56 and 57

Later it combinded insn 57 -> 58 to the expression that has
triggered the bad register allocation.  This looks all correct to
me.

> Even if subregs are allowed, paradoxical subregs look really odd here.
> 
> The ICE'ing test case, reload comes up with
> 
> (insn 97 57 98 9 (set (reg:HI 30 r30)
> (reg/v/f:HI 20 r20 [orig:75 operands ] [75])) "pr26833.c":11
> 68 {*movhi}

Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-22 Thread Georg-Johann Lay

On 21.12.2016 18:46, Segher Boessenkool wrote:

On Wed, Dec 21, 2016 at 01:58:18PM +0100, Georg-Johann Lay wrote:

$ 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)




Combine comes up with the following insn:
(jump_insn 58 57 59 7 (set (pc)
(if_then_else (eq (and:HI (subreg:HI (mem:QI (reg/v/f:HI 75 [
operands ]) [1 *operands_17(D)+0 S1 A8]) 0)
(const_int 1 [0x1]))
(const_int 0 [0]))
(label_ref 70)
(pc)))
"/home/georg/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)

which cannot be correct because avr.md::*sbrx_and_branchhi reads:

(define_insn "*sbrx_and_branch"
  [(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)))]
  "" { ... } ...)

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.


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.

For a simpler test case like the following

unsigned u;
int volatile v;

void btest1 (void)
{
if (u & 1)
v = 0;
v = 0;
}

the combine pattern is also used but the register pressure is smaller.
After quite some spills, reload then comes up with

(insn 18 7 8 2 (set (reg:QI 24 r24)
(mem/c:QI (symbol_ref:HI ("u")  ) [1 
u+0 S1 A8])) "testbit.c":6 56 {movqi_insn}

 (nil))
(jump_insn 8 18 9 2 (set (pc)
(if_then_else (eq (and:HI (reg:HI 24 r24)
(const_int 1 [0x1]))
(const_int 0 [0]))
(label_ref 11)
(pc))) "testbit.c":6 415 {*sbrx_and_branchhi}
 (int_list:REG_BR_PROB 4600 (nil))
 -> 11)

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?

Even if subregs are allowed, paradoxical subregs look really odd here.

The ICE'ing test case, reload comes up with

(insn 97 57 98 9 (set (reg:HI 30 r30)
(reg/v/f:HI 20 r20 [orig:75 operands ] [75])) "pr26833.c":11 68 
{*movhi}

 (nil))
(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?

As far as I know, reload generates only a very limited subset of insns,
namely additions to compute frame addresses and mov insns.  But
reload is not supposed to generate extends or other arithmetic.

If the purpose of the change to combine.c was to optimize code,
then it would be preferred to avoid HI altogether and just use QImode.
avr.md has a similar insn for QI, so that would really give a
reasonable match.

Johann



Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-21 Thread Segher Boessenkool
On Wed, Dec 21, 2016 at 01:58:18PM +0100, Georg-Johann Lay wrote:
> $ 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)


> Combine comes up with the following insn:
> (jump_insn 58 57 59 7 (set (pc)
> (if_then_else (eq (and:HI (subreg:HI (mem:QI (reg/v/f:HI 75 [ 
> operands ]) [1 *operands_17(D)+0 S1 A8]) 0)
> (const_int 1 [0x1]))
> (const_int 0 [0]))
> (label_ref 70)
> (pc))) 
> "/home/georg/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)
> 
> which cannot be correct because avr.md::*sbrx_and_branchhi reads:
> 
> (define_insn "*sbrx_and_branch"
>   [(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)))]
>   "" { ... } ...)
> 
> 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.

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?


Segher


Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-21 Thread Georg-Johann Lay

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"
  [(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 ^_^  ^_^





Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-21 Thread Dominik Vogt
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))

?

> /gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/compile/pr26833.c:19:1:
> internal compiler error: in extract_constrain_insn, at recog.c:2213
> 0x9836a3 _fatal_insn(char const*, rtx_def const*, char const*, int,
> char const*)
>   ../../../gcc.gnu.org/trunk/gcc/rtl-error.c:108
> 0x9836cf _fatal_insn_not_found(rtx_def const*, char const*, int,
> char const*)
>   ../../../gcc.gnu.org/trunk/gcc/rtl-error.c:119
> 0x95abdd extract_constrain_insn(rtx_insn*)
>   ../../../gcc.gnu.org/trunk/gcc/recog.c:2213
> 0x939105 reload_cse_simplify_operands
>   ../../../gcc.gnu.org/trunk/gcc/postreload.c:391
> 0x939ce5 reload_cse_simplify
>   ../../../gcc.gnu.org/trunk/gcc/postreload.c:179
> 0x939ce5 reload_cse_regs_1
>   ../../../gcc.gnu.org/trunk/gcc/postreload.c:218
> 0x93b96b reload_cse_regs
>   ../../../gcc.gnu.org/trunk/gcc/postreload.c:64
> 0x93b96b execute
>   ../../../gcc.gnu.org/trunk/gcc/postreload.c:2342
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See  for instructions.
> 
> 
> Target: avr
> Configured with: ../../gcc.gnu.org/trunk/configure --target=avr
> --prefix=/local/gnu/install/gcc-7 --disable-shared --disable-nls
> --with-dwarf2 --enable-target-optspace=yes --with-gnu-as
> --with-gnu-ld --enable-checking=release --enable-languages=c,c++
> 
> 
> Combine comes up with the following insn:
> (jump_insn 58 57 59 7 (set (pc)
> (if_then_else (eq (and:HI (subreg:HI (mem:QI (reg/v/f:HI 75
> [ operands ]) [1 *operands_17(D)+0 S1 A8]) 0)
> (const_int 1 [0x1]))
> (const_int 0 [0]))
> (label_ref 70)
> (pc))) 
> "/home/georg/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)
> 
> which cannot be correct because avr.md::*sbrx_and_branchhi reads:
> 
> (define_insn "*sbrx_and_branch"
>   [(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)))]
>   "" { ... } ...)
> 
> 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 ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-21 Thread Georg-Johann Lay

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)
/gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/compile/pr26833.c:19:1: 
internal compiler error: in extract_constrain_insn, at recog.c:2213
0x9836a3 _fatal_insn(char const*, rtx_def const*, char const*, int, char 
const*)

../../../gcc.gnu.org/trunk/gcc/rtl-error.c:108
0x9836cf _fatal_insn_not_found(rtx_def const*, char const*, int, char 
const*)

../../../gcc.gnu.org/trunk/gcc/rtl-error.c:119
0x95abdd extract_constrain_insn(rtx_insn*)
../../../gcc.gnu.org/trunk/gcc/recog.c:2213
0x939105 reload_cse_simplify_operands
../../../gcc.gnu.org/trunk/gcc/postreload.c:391
0x939ce5 reload_cse_simplify
../../../gcc.gnu.org/trunk/gcc/postreload.c:179
0x939ce5 reload_cse_regs_1
../../../gcc.gnu.org/trunk/gcc/postreload.c:218
0x93b96b reload_cse_regs
../../../gcc.gnu.org/trunk/gcc/postreload.c:64
0x93b96b execute
../../../gcc.gnu.org/trunk/gcc/postreload.c:2342
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.


Target: avr
Configured with: ../../gcc.gnu.org/trunk/configure --target=avr 
--prefix=/local/gnu/install/gcc-7 --disable-shared --disable-nls 
--with-dwarf2 --enable-target-optspace=yes --with-gnu-as --with-gnu-ld 
--enable-checking=release --enable-languages=c,c++



Combine comes up with the following insn:
(jump_insn 58 57 59 7 (set (pc)
(if_then_else (eq (and:HI (subreg:HI (mem:QI (reg/v/f:HI 75 [ 
operands ]) [1 *operands_17(D)+0 S1 A8]) 0)

(const_int 1 [0x1]))
(const_int 0 [0]))
(label_ref 70)
(pc))) 
"/home/georg/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)

which cannot be correct because avr.md::*sbrx_and_branchhi reads:

(define_insn "*sbrx_and_branch"
  [(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)))]
  "" { ... } ...)

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




Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-12 Thread Dominik Vogt
On Mon, Dec 12, 2016 at 10:54:12AM -0600, 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?

That was triggered by several regression test cases, e.g.
/gcc.c-torture/compile/pr57108.c.  The LSHIFTRT had VOIDmode.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-12 Thread Segher Boessenkool
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.


Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2016-12-12 Thread Dominik Vogt
On Sat, Dec 10, 2016 at 10:37:38AM -0600, Segher Boessenkool wrote:
> On Fri, Dec 09, 2016 at 04:23:44PM +0100, Dominik Vogt wrote:
> > 0001-*
> > 
> >   Deal with mode expanding zero_extracts in change_zero_ext.  The
> >   patch looks good to me, but not sure whether endianness is
> >   handled properly.  Is the second argument of gen_rtx_SUBREG
> >   correct?
> 
> > >From 600ed3dadd5bc2568ab53be8466686abaf27ff3f Mon Sep 17 00:00:00 2001
> > From: Dominik Vogt 
> > Date: Fri, 9 Dec 2016 02:48:30 +0100
> > Subject: [PATCH 1/2] combine: Handle mode expanding zero_extracts in
> >  change_zero_ext.
> > 
> > Example:
> > 
> >   (zero_extract:DI (reg:SI)
> >(const_int 24)
> >(const_int 0))
> > 
> > -->
> > 
> >   (and:DI (subreg:DI (lshiftrt:SI (reg:SI) (const_int 8))
> >  0)
> >   (const_int 16777215))
> > ---
> >  gcc/combine.c | 12 +---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/gcc/combine.c b/gcc/combine.c
> > index b429453..e14a08f 100644
> > --- a/gcc/combine.c
> > +++ b/gcc/combine.c
> > @@ -11237,18 +11237,24 @@ change_zero_ext (rtx pat)
> >if (GET_CODE (x) == ZERO_EXTRACT
> >   && CONST_INT_P (XEXP (x, 1))
> >   && CONST_INT_P (XEXP (x, 2))
> > - && GET_MODE (XEXP (x, 0)) == mode)
> > + && (GET_MODE (XEXP (x, 0)) == mode
> > + || GET_MODE_PRECISION (GET_MODE (XEXP (x, 0)))
> > +< GET_MODE_PRECISION (mode)))
> 
> If you make this <= you can collapse the two cases into one.
> 
> > {
> > + machine_mode inner_mode = GET_MODE (XEXP (x, 0));
> > +
> >   size = INTVAL (XEXP (x, 1));
> >  
> >   int start = INTVAL (XEXP (x, 2));
> >   if (BITS_BIG_ENDIAN)
> > -   start = GET_MODE_PRECISION (mode) - size - start;
> > +   start = GET_MODE_PRECISION (inner_mode) - size - start;
> >  
> >   if (start)
> > -   x = gen_rtx_LSHIFTRT (mode, XEXP (x, 0), GEN_INT (start));
> > +   x = gen_rtx_LSHIFTRT (inner_mode, XEXP (x, 0), GEN_INT (start));
> >   else
> > x = XEXP (x, 0);
> > + if (mode != inner_mode)
> > +   x = gen_rtx_SUBREG (mode, x, 0);
> 
> gen_lowpart_SUBREG instead?  It's easier to read, and code a little
> further down does the same thing.
> 
> Okay for trunk with those changes (did you bootstrap+regcheck this
> already?)  Thanks,

Patch with these changes and a fix because of not handling
VOIDmode attached.  Bootstrapped and regression tested on s390 and
s390x.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany
gcc/ChangeLog-change_zero_ext-1

* combine.c (change_zero_ext): Handle mode expanding zero_extracts.
>From 0c23906cb81c745571903799a00338fe974303da Mon Sep 17 00:00:00 2001
From: Dominik Vogt 
Date: Fri, 9 Dec 2016 02:48:30 +0100
Subject: [PATCH] combine: Handle mode expanding zero_extracts in
 change_zero_ext.

Example:

  (zero_extract:DI (reg:SI)
   (const_int 24)
   (const_int 0))

-->

  (and:DI (subreg:DI (lshiftrt:SI (reg:SI) (const_int 8))
 0)
  (const_int 16777215))
---
 gcc/combine.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/gcc/combine.c b/gcc/combine.c
index b429453..19851a2 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -11237,18 +11237,24 @@ change_zero_ext (rtx pat)
   if (GET_CODE (x) == ZERO_EXTRACT
  && CONST_INT_P (XEXP (x, 1))
  && CONST_INT_P (XEXP (x, 2))
- && GET_MODE (XEXP (x, 0)) == mode)
+ && GET_MODE (XEXP (x, 0)) != VOIDmode
+ && GET_MODE_PRECISION (GET_MODE (XEXP (x, 0)))
+ <= GET_MODE_PRECISION (mode))
{
+ machine_mode inner_mode = GET_MODE (XEXP (x, 0));
+
  size = INTVAL (XEXP (x, 1));
 
  int start = INTVAL (XEXP (x, 2));
  if (BITS_BIG_ENDIAN)
-   start = GET_MODE_PRECISION (mode) - size - start;
+   start = GET_MODE_PRECISION (inner_mode) - size - start;
 
  if (start)
-   x = gen_rtx_LSHIFTRT (mode, XEXP (x, 0), GEN_INT (start));
+   x = gen_rtx_LSHIFTRT (inner_mode, XEXP (x, 0), GEN_INT (start));
  else
x = XEXP (x, 0);
+ if (mode != inner_mode)
+   x = gen_lowpart_SUBREG (mode, x);
}
   else if (GET_CODE (x) == ZERO_EXTEND
   && SCALAR_INT_MODE_P (mode)
-- 
2.3.0