| Message ID | CAMqJFCqWDNTjmJ11p-LGE4vBPQ7gBSLWY3prXFNv7+QUoL_ipw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Nov 8, 2013 at 3:40 PM, Joern Rennecke
<joern.rennecke@embecosm.com> wrote:
> bootstrapped / regtested on i686-pc-linux-gnu.
Not a very elaborate description of the patch, eh? :-)
This is IMHO not OK without at least an explanation of why the
comparison of two const_ints is not folded. Better yet would be to fix
that underlying problem. We should not present such non-sense to the
RTL parts of the middle end.
Also would need a test case.
Ciao!
Steven
On 8 November 2013 14:45, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> Also would need a test case.
As is mentioned in the PR, we already have a test case, which shows a
regression.
On 8 November 2013 14:45, Steven Bosscher <stevenb.gcc@gmail.com> wrote: > This is IMHO not OK without at least an explanation of why the > comparison of two const_ints is not folded. Better yet would be to fix > that underlying problem. We should not present such non-sense to the > RTL parts of the middle end. Which part would be responsible to folding the comparison at -O1 ? FWIW, as I just commented in the PR, one of the operands is an ssa_name with a known value.
On Fri, Nov 8, 2013 at 4:18 PM, Joern Rennecke wrote: > On 8 November 2013 14:45, Steven Bosscher wrote: >> This is IMHO not OK without at least an explanation of why the >> comparison of two const_ints is not folded. Better yet would be to fix >> that underlying problem. We should not present such non-sense to the >> RTL parts of the middle end. > > Which part would be responsible to folding the comparison at -O1 ? > FWIW, as I just commented in the PR, one of the operands is an ssa_name > with a known value. This usually is taken care of by one of the many propagation passes (CCP, VRP, forwprop, DOM, even FRE/PRE). The only cases I've previously encountered, where a known value ends up not being propagated, is when the propagation is somehow overlooked and the known value is only replaced at out-of-ssa time, i.e. TER. The only way to find out where the propagation should have happened, is by looking at the gimple dumps. Ciao! Steven
On Fri, Nov 08, 2013 at 04:32:09PM +0100, Steven Bosscher wrote: > On Fri, Nov 8, 2013 at 4:18 PM, Joern Rennecke wrote: > > On 8 November 2013 14:45, Steven Bosscher wrote: > >> This is IMHO not OK without at least an explanation of why the > >> comparison of two const_ints is not folded. Better yet would be to fix > >> that underlying problem. We should not present such non-sense to the > >> RTL parts of the middle end. > > > > Which part would be responsible to folding the comparison at -O1 ? > > FWIW, as I just commented in the PR, one of the operands is an ssa_name > > with a known value. > > This usually is taken care of by one of the many propagation passes > (CCP, VRP, forwprop, DOM, even FRE/PRE). The only cases I've > previously encountered, where a known value ends up not being > propagated, is when the propagation is somehow overlooked and the > known value is only replaced at out-of-ssa time, i.e. TER. > > The only way to find out where the propagation should have happened, > is by looking at the gimple dumps. Well, most of the passes you've mentioned can be disabled with -fno-tree-* but still we shouldn't ICE on it, so the expander can't assume that it will never see something like that, only doesn't have to try too hard to optimize that unlikely case. Of course, it is nice to know why it hasn't been optimized in the particular case. Jakub
On Fri, Nov 8, 2013 at 4:41 PM, Jakub Jelinek wrote: > On Fri, Nov 08, 2013 at 04:32:09PM +0100, Steven Bosscher wrote: >> On Fri, Nov 8, 2013 at 4:18 PM, Joern Rennecke wrote: >> > On 8 November 2013 14:45, Steven Bosscher wrote: >> >> This is IMHO not OK without at least an explanation of why the >> >> comparison of two const_ints is not folded. Better yet would be to fix >> >> that underlying problem. We should not present such non-sense to the >> >> RTL parts of the middle end. >> > >> > Which part would be responsible to folding the comparison at -O1 ? >> > FWIW, as I just commented in the PR, one of the operands is an ssa_name >> > with a known value. >> >> This usually is taken care of by one of the many propagation passes >> (CCP, VRP, forwprop, DOM, even FRE/PRE). The only cases I've >> previously encountered, where a known value ends up not being >> propagated, is when the propagation is somehow overlooked and the >> known value is only replaced at out-of-ssa time, i.e. TER. >> >> The only way to find out where the propagation should have happened, >> is by looking at the gimple dumps. > > Well, most of the passes you've mentioned can be disabled with -fno-tree-* > but still we shouldn't ICE on it, so the expander can't assume that > it will never see something like that, only doesn't have to try too hard > to optimize that unlikely case. Of course, it is nice to know why it hasn't > been optimized in the particular case. Even with the gimple opts disabled, a const-const comparison would normally be folded by the RTL expanders. Ciao! Steven
On 11/08/13 07:45, Steven Bosscher wrote: > On Fri, Nov 8, 2013 at 3:40 PM, Joern Rennecke > <joern.rennecke@embecosm.com> wrote: >> bootstrapped / regtested on i686-pc-linux-gnu. > > Not a very elaborate description of the patch, eh? :-) > > This is IMHO not OK without at least an explanation of why the > comparison of two const_ints is not folded. Better yet would be to fix > that underlying problem. We should not present such non-sense to the > RTL parts of the middle end. Agreed. jeff
Index: gcc/expmed.c =================================================================== --- gcc/expmed.c (revision 204568) +++ gcc/expmed.c (working copy) @@ -5112,6 +5112,12 @@ emit_cstore (rtx target, enum insn_code if (!target) target = gen_reg_rtx (target_mode); + /* If we compare two VOIDmode constants, we loose the mode to do the + comparison in. To avoid that, copy one constant into a register. + We rely on subsequent optimizations (like ce1) to clean this up. */ + if (GET_MODE (x) == VOIDmode && GET_MODE (y) == VOIDmode) + x = copy_to_mode_reg (compare_mode, x); + comparison = gen_rtx_fmt_ee (code, result_mode, x, y); create_output_operand (&ops[0], optimize ? NULL_RTX : target, result_mode);