Re: [PATCH] Improve combining of conditionals
On Apr 15, 2011, at 4:38 PM, Maxim Kuvyrkov wrote: On Apr 15, 2011, at 3:34 PM, Eric Botcazou wrote: The problem this patch fixes is that combine_simplify_rtx() prefers to return an expression (say, xor (a) (b)) even when a comparison is prefered (say, neq (xor (a) (b)) 0). Expressions are not recognized as valid conditions of if_then_else for most targets, so combiner misses a potential optimization. This patch makes combine_simplify_rtx() aware of the context it was invoked in, and, when appropriate, does not discourage it from returning a conditional. Btw, this is very likely also valid for targets with STORE_FLAG_VALUE == -1 so the same IN_COND short-circuit would need to be added a few lines below in combine_simplify_rtx. But this would need to be tested. Do you happen to have access to such a target, e.g. m68k? Hm, I didn't notice that one, thanks! I have access to m68k (ColdFire, tbp) and will test this change there before committing. I've successfully tested the following trivial patch on m68k-linux (cross-compiler, gcc, g++ and libstdc++ testsuites). Checked in as obvious. -- Maxim Kuvyrkov Mentor Graphics / CodeSourcery fsf-gcc-combine-fix-1.ChangeLog Description: Binary data fsf-gcc-combine-fix-1.patch Description: Binary data
[PATCH] Improve combining of conditionals
This patch fixes the problem with substituting expressions into IF_THEN_ELSE
during combining. Without this patch combining of conditionals inside
IF_THEN_ELSE is seriously inhibited.
The problem this patch fixes is that combine_simplify_rtx() prefers to return
an expression (say, xor (a) (b)) even when a comparison is prefered (say,
neq (xor (a) (b)) 0). Expressions are not recognized as valid conditions of
if_then_else for most targets, so combiner misses a potential optimization.
This patch makes combine_simplify_rtx() aware of the context it was invoked in,
and, when appropriate, does not discourage it from returning a conditional.
The motivating example for this fix was crcu8() routine from CoreMark.
Compiling this routine for MIPS32R2 at -O3 produces there are several instances
of sequence
andi$2,$2,0x1
xori$2,$2,0x1
movn$3,$5,$2 ; $2 dies here
which can be optimized into
andi$2,$2,0x1
movz$3,$5,$2 ; $2 dies here
.
The patch was successfully tested on {i686, arm, mips}-linux, both GCC
testsuites and SPEC2000 runs. For all targets there was no observable code
difference in SPEC2000 benchmarks, so the example does not trigger very often.
Still, it speeds up CoreMark by about 1%.
OK for trunk?
--
Maxim Kuvyrkov
Mentor Graphics / CodeSourcery
gcc-combine-if_then_else.ChangeLog
Description: Binary data
gcc-combine-if_then_else.patch
Description: Binary data
Re: [PATCH] Improve combining of conditionals
The patch was successfully tested on {i686, arm, mips}-linux, both GCC
testsuites and SPEC2000 runs. For all targets there was no observable code
difference in SPEC2000 benchmarks, so the example does not trigger very
often. Still, it speeds up CoreMark by about 1%.
OK for trunk?
Yes, modulo the following nits:
@@ -4938,11 +4938,13 @@ find_split_point (rtx *loc, rtx insn, bool set_src)
IN_DEST is nonzero if we are processing the SET_DEST of a SET.
+ IN_COND is nonzero if we are on top level of the condition.
...we are at the top level of a condition.
@@ -5221,10 +5225,12 @@ subst (rtx x, rtx from, rtx to, int in_dest, int
unique_copy)
expression.
OP0_MODE is the original mode of XEXP (x, 0). IN_DEST is nonzero
- if we are inside a SET_DEST. */
+ if we are inside a SET_DEST. IN_COND is nonzero if we are on the top level
+ of a condition. */
Likewise.
@@ -5717,7 +5723,16 @@ combine_simplify_rtx (rtx x, enum machine_mode op0_mode,
int in_dest)
ZERO_EXTRACT is indeed appropriate, it will be placed back by
the call to make_compound_operation in the SET case. */
- if (STORE_FLAG_VALUE == 1
+ if (in_cond)
+ /* Don't apply below optimizations if the caller would
+ prefer a comparison rather than a value.
+ E.g., for the condition in an IF_THEN_ELSE most targets need
+ an explicit comparison. */
+ {
+ ;
+ }
Remove the superfluous parentheses and move the comment to a new paragraph of
the big comment just above.
No need to retest, just make sure this still compiles, thanks in advance.
--
Eric Botcazou
Re: [PATCH] Improve combining of conditionals
And make IN_COND a bool instead of an int? I had the same initial reaction but then came to the same conclusion as Maxim. -- Eric Botcazou
Re: [PATCH] Improve combining of conditionals
On Apr 15, 2011, at 3:34 PM, Eric Botcazou wrote: The problem this patch fixes is that combine_simplify_rtx() prefers to return an expression (say, xor (a) (b)) even when a comparison is prefered (say, neq (xor (a) (b)) 0). Expressions are not recognized as valid conditions of if_then_else for most targets, so combiner misses a potential optimization. This patch makes combine_simplify_rtx() aware of the context it was invoked in, and, when appropriate, does not discourage it from returning a conditional. Btw, this is very likely also valid for targets with STORE_FLAG_VALUE == -1 so the same IN_COND short-circuit would need to be added a few lines below in combine_simplify_rtx. But this would need to be tested. Do you happen to have access to such a target, e.g. m68k? Hm, I didn't notice that one, thanks! I have access to m68k (ColdFire, tbp) and will test this change there before committing. -- Maxim Kuvyrkov Mentor Graphics / CodeSourcery
Re: [PATCH] Improve combining of conditionals
On Apr 15, 2011, at 3:44 AM, Eric Botcazou ebotca...@adacore.com wrote: And make IN_COND a bool instead of an int? I had the same initial reaction but then came to the same conclusion as Maxim. If it can be bool, it should be bool.
Re: [PATCH] Improve combining of conditionals
If it can be bool, it should be bool. Rather basic principle IMO. Consistency is of much greater value. -- Eric Botcazou
