This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix incorrect optab expansion for ulcmp on ARM.


On Fri, 27 Jan 2006, Carlos O'Donell wrote:
> 2006-01-27  Carlos O'Donell  <carlos@codesourcery.com>
>
> 	* optabs.c (prepare_cmp_insn): If unbaised and unsigned then bias
> 	the comparison routine return.
>
> 	* gcc.dg/unsigned-long-compare.c: New test.

This is OK for both mainline and the 4.1 branch.

I'm a bit uncomfortable that there's still something wrong with the
design of this code.  compare_from_rtx's use of emit_cmp_insn only
seems safe for cc0 targets.  Even with your fix to prepare_cmp_insn,
we still ignore the change to *pmode with integer comparisons, and
also ignore the changes in prepare_float_lib_cmp.  compare_from_rtx
could be moved from do_jump.c to optabs.c and call directly to
prepare_cmp_insn and emit_cmp_and_jump_insn_1, but how emit_cmp_insn
is supposed to work is a mystery.

Despite my concerns that this papering over a fundamental design
issue, your patch is an improvement of current mainline, it does
resolve the regression in your testcase, and could only affect
the ARM.  It undoubtedly the right thing to do for 4.1, but I'd
appreciate it if we could investigate some follow-up improvements.

There may also be potential optimizer benefits of allowing
prepare_cmp_insn to use equality/inequality comparisons instead
of LT, GT, LTU, GTU so that the result value may be CSE'd into
the following code.  i.e. "result == 2" has more information
content than "result > 1" during the following RTL optimizers.


Thanks for investigating.  The new comment is particularly informative.

Roger
--


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]