From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27616 invoked by alias); 9 Nov 2012 13:29:30 -0000 Received: (qmail 27550 invoked by uid 48); 9 Nov 2012 13:29:13 -0000 From: "amylaar at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug target/54089] [SH] Refactor shift patterns Date: Fri, 09 Nov 2012 13:29:00 -0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: target X-Bugzilla-Keywords: X-Bugzilla-Severity: enhancement X-Bugzilla-Who: amylaar at gcc dot gnu.org X-Bugzilla-Status: ASSIGNED X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: olegendo at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated Content-Type: text/plain; charset="UTF-8" MIME-Version: 1.0 Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-bugs-owner@gcc.gnu.org X-SW-Source: 2012-11/txt/msg00827.txt.bz2 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54089 --- Comment #27 from Jorn Wolfgang Rennecke 2012-11-09 13:29:10 UTC --- (In reply to comment #26) > (In reply to comment #25) > > Maybe the better solution would indeed be to add a > > arith -> logical shift conversion pass before combine, or try to convert arith > > shifts in the split pass after combine by looking at the data flow of the > > shifted values. FWIW, while the ARC600/700 have a full set up static and dynamic shifts, the ARC 601 with the swap option has similar issues with shifts that should be stitched together from multiple instructions, and preferences of unsigned over signed shifts. > Another idea would be to extend the combine pass, so that whenever it > internally converts arith -> logical shifts, it always checks the costs of the > shift op, and if the logical shift is cheaper, always split out the logical > shift. Indeed, combine's insistence that a computation has just one canonical form, which depends on how much it known about the input/output data, is in want of some correction. We got define_split for things that should become multiple instructions, but nothing for things that should just be differently-shaped instructions, and there are no predicates available to get to know about the (non)zero / signbits that combine knows about. E.g. the following peephole in arc.md is only necessary because combine does some unwanted 'simplifications' with known-zero bits: ;; ------------------------------------------------------------- ;; Pattern 1 : r0 = r1 << {i} ;; r3 = r4/INT + r0 ;;and commutative ;; || ;; \/ ;; add{i} r3,r4/INT,r1 ;; ------------------------------------------------------------- ;; ??? This should be covered by combine, alas, at times combine gets ;; too clever for it's own good: when the shifted input is known to be ;; either 0 or 1, the operation will be made into an if-then-else, and ;; thus fail to match the add_n pattern. Example: _mktm_r, line 85 in ;; newlib/libc/time/mktm_r.c . (define_peephole2 [(set (match_operand:SI 0 "dest_reg_operand" "") (ashift:SI (match_operand:SI 1 "register_operand" "") (match_operand:SI 2 "const_int_operand" ""))) (set (match_operand:SI 3 "dest_reg_operand" "") (plus:SI (match_operand:SI 4 "nonmemory_operand" "") (match_operand:SI 5 "nonmemory_operand" "")))] "(INTVAL (operands[2]) == 1 || INTVAL (operands[2]) == 2 || INTVAL (operands[2]) == 3) && (true_regnum (operands[4]) == true_regnum (operands[0]) || true_regnum (operands[5]) == true_regnum (operands[0])) && (peep2_reg_dead_p (2, operands[0]) || (true_regnum (operands[3]) == true_regnum (operands[0])))" ;; the preparation statements take care to put proper operand in operands[4] ;; operands[4] will always contain the correct operand. This is added to satisfy commutativity [(set (match_dup 3) (plus:SI (mult:SI (match_dup 1) (match_dup 2)) (match_dup 4)))] "if (true_regnum (operands[4]) == true_regnum (operands[0])) operands[4] = operands[5]; operands[2] = GEN_INT (1 << INTVAL (operands[2]));" )