Median of three has unneeded register moves
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| Linaro GCC |
Triaged
|
Medium
|
Unassigned | ||
Bug Description
This code is from libav mathops.h:
/* Return the median of 3 */
int mid_pred(int a, int b, int c)
{
if(a>b){
if(c>b){
if(c>a) b=a;
else b=c;
}
}else{
if(b>c){
if(c>a) b=c;
else b=a;
}
}
return b;
}
The assembly generated with gcc-linaro-
mid_pred:
cmp r0, r1
ble .L2
cmp r1, r2
bge .L3
cmp r2, r0
movlt r1, r2
movge r1, r0
.L3:
mov r0, r1
bx lr
.L2:
cmp r1, r2
ble .L3
cmp r2, r0
movge r1, r2
movlt r1, r0
mov r0, r1
bx lr
This is OK, but there are some unneeded moves that could be avoided if the result was loaded directly into r0 such as:
mid_pred:
cmp r0, r1
ble .L2
cmp r1, r2
bge .L4
cmp r2, r0
movlt r0, r2
bx lr
.L4:
mov r0, r1
bx lr
.L2:
cmp r1, r2
ble .L3
cmp r2, r0
movge r0, r2
bx lr
| Changed in gcc-linaro: | |
| status: | Triaged → New |
| Ramana Radhakrishnan (ramana) wrote | #1 |
| Changed in gcc-linaro: | |
| status: | New → Triaged |
| importance: | Undecided → Medium |
| Ramana Radhakrishnan (ramana) wrote | #2 |
| Christophe Lyon (christophe-lyon) wrote | #3 |
We have an smax instruction that expands the form:
(define_insn "*arm_smax_insn"
[(set (match_operand:SI 0 "s_register_
(smax:SI (match_operand:SI 1 "s_register_
(clobber (reg:CC CC_REGNUM))]
"TARGET_ARM"
"@
cmp\\t%1, %2\;movlt\\t%0, %2
cmp\\t%1, %2\;movge\\t%0, %1\;movlt\\t%0, %2"
[(set_attr "conds" "clob")
(set_attr "length" "8,12")]
)
We could in theory split this into multiple instructions after reload ( the cmp followed by a movsicc) - however this doesn't really help in this case - The problem with this testcase is that we have a very simple form of smax and smin where given that the expression isn't complex enough reload doesn't kick in or the register allocator doesn't end up noticing that the result well is actually used in r0. We could hack a fix with a peephole 2 that removed such moves to the result register but that's just faking it !
I'm not sure how to proceed further on this one without looking further into reload.