Bug 10982 - [arm] poor optimisation of "if (var & const)"
Summary: [arm] poor optimisation of "if (var & const)"
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 3.4.0
: P2 enhancement
Target Milestone: 4.0.0
Assignee: Richard Earnshaw
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2003-05-26 13:53 UTC by Philip Blundell
Modified: 2004-05-16 22:24 UTC (History)
2 users (show)

See Also:
Host:
Target: arm-linux
Build:
Known to work:
Known to fail:
Last reconfirmed: 2004-05-16 22:19:36


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Blundell 2003-05-26 13:53:10 UTC
Compiling this code:

f(int a)
{
  if (a & 0xfff)
    return 1;
  return 0;
}

with -O2 yields:

        mov     r0, r0, asl #20
        mov     r0, r0, lsr #20
        cmp     r0, #0
        movne   r0, #1
        moveq   r0, #0

There's no need to fully calculate the value of the AND, since all that we care
about is whether it's zero or not.  So, I think the first three instructions
could be replaced by a single one:

        movs     r0, r0, asl #20
Comment 1 Richard Earnshaw 2003-05-29 11:11:24 UTC
Confirmed.  In fact, in this instance the entire sequence can be collapsed to

	movs	r0, r0, asl #20
	movne	r0, #1
Comment 2 Philip Blundell 2004-05-09 17:11:28 UTC
For that particular testcase, adding a pattern like the one below seems to allow
combine to do the right thing.  However, changing the "return 1" to "return 2"
prevents this from matching.  Something a bit more sophisticated might be needed.

--

(define_insn_and_split "*zero_extract_compare0_shifted"
  [(set (match_operand:SI 0 "s_register_operand" "=r")
	(ne:SI (zero_extract:SI (match_operand:SI 1 "s_register_operand" "r")
		        	   (match_operand:SI 2 "const_int_operand" "n")
			      	   (const_int 0))
	            	  (const_int 0)))
   (clobber (reg:CC CC_REGNUM))]
  "TARGET_ARM"
  "#"
  ""
  [(parallel [(set (reg:CC_NOOV CC_REGNUM)
		   (compare:CC_NOOV (ashift:SI (match_dup 1)
			                       (match_dup 2))
			            (const_int 0)))
	      (set (match_dup 0)
		   (ashift:SI (match_dup 1) (match_dup 2)))])
   (set (match_dup 0)
        (if_then_else (eq:SI (reg:CC_NOOV CC_REGNUM) (const_int 0))
	              (match_dup 0)
		      (const_int 1)))]
  ""
)

;; Match the second half of *zero_extract_compare0_shifted
(define_insn "*movsi_conditional"
  [(set (match_operand:SI 0 "s_register_operand" "=r")
        (if_then_else (eq:SI (reg:CC_NOOV CC_REGNUM) (const_int 0))
	              (match_dup 0)
		      (match_operand:SI 1 "const_int_operand" "n")))]
  "TARGET_ARM"
  "movne\\t%0, %1"
  [(set_attr "conds" "use")]
)
Comment 3 Richard Earnshaw 2004-05-16 22:19:36 UTC
The proposed patch isn't quite correct.  There are already patterns that can be
used as the target of the split, so there's no need to add new ones, and the
split needs to 

1) Convert operand2
2) Not apply to Thumb state


Patch forthcoming.
Comment 4 CVS Commits 2004-05-16 22:22:55 UTC
Subject: Bug 10982

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	rearnsha@gcc.gnu.org	2004-05-16 22:22:49

Modified files:
	gcc            : ChangeLog 
	gcc/config/arm : arm.md 

Log message:
	PR target/10982
	* arm.md (ne_zeroextractsi): Convert to insn-and-split.
	(ne_zeroextractsi_shifted): New pattern.
	(ite_ne_zeroextractsi): New pattern.
	(ite_ne_zeroextractsi_shifted): New pattern.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.3669&r2=2.3670
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/arm/arm.md.diff?cvsroot=gcc&r1=1.167&r2=1.168

Comment 5 Richard Earnshaw 2004-05-16 22:23:50 UTC
Fixed with applied patch