i386.md bug + fix
Jim Wilson
wilson@cygnus.com
Thu May 7 20:50:00 GMT 1998
int
foo (char* s)
{
return strlen (s);
}
You will notice several pages of assembler output rather
than the required four or so lines.
This is a different problem than the first one you reported. In this
case, gcc is deliberately emitting 39 instructions instead of 4, because
these 39 instrutions are faster. This would only be a bug if this code
was slower, and you haven't reported that.
If you want smaller code instead of faster code, then we could add an
option to optimize for space instead of speed, but you haven't asked for
that either.
(insn 13 12 15 (parallel[
(set (reg:SI 22)
(unspec:SI[
(mem:BLK (reg:SI 22)) // Pointer
(const_int 1) // This should be e-o-s
(reg:SI 23) // Alignment: should be 1
] 0))
(clobber (reg:SI 23))
] ) -1 (nil)
(nil))
This is wrong. strlensi and strlensi+1 use the operands in this order
output/pointer/e-o-s/align. However, this particular RTL matches the
strlensi_unroll4 pattern which takes a different set of operands.
Similarly for strlensi_unroll5. For these patterns, the operands are
output/pointer/alignment/scratch. Hence, the RTL is exactly correct.
See the comments before output_strlen_unroll in i386.c.
The fact that it doesn't go looking for a string
that ends with 1 is because that param isn't ever actually
explicitly refered to or USEd anywhere
Again there is this comment about lack of a USE rtl, but a USE does not
perform any action, it is simply a placeholder so that we can force a
pattern to have an explicit reference to a REG (or something). In this
case, the pattern already has an explicit reference to the operand in
question inside the UNSPEC rtl, so adding a USE rtl does not do anything
useful.
(insn 13 12 14 (parallel[
(set (reg:SI 25)
(unspec:SI[
(mem:BLK (reg:SI 24)) // Pointer
(reg:QI 23) // The e-o-s
(const_int 1) // Alignment
] 0))
(use (reg:QI 23))
(clobber (reg:SI 24))
] ) -1 (nil)
(nil))
Yes, we get different RTL with your patch, but note here that we are now
matching a different pattern. This is now the strlensi+1 pattern, which
takes different operands than strlensi_unroll5, and that is why the REG and
CONST_INT have switched positions.
Your patch hasn't fixed anything, but rather it has actually broken something.
Because you changed the operand2 predicate from immediate_operand to
register_operand, operand2 can never be a const_int 0, and hence it is
impossible for the strlen unrolling code to ever be called. This is why
we are suddenly getting different RTL. This is actually a bug, because we
should generate the strlen unrolling code for this case.
This further supports my claim that your patch is incorrect.
If you supply a testcase for the bug that causes kaffe to be miscompiled,
I will work on a better patch.
Jim
More information about the Gcc-bugs
mailing list