From patchwork Mon Sep 5 17:07:33 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Stubbs X-Patchwork-Id: 113407 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 457FCB6F7D for ; Tue, 6 Sep 2011 03:08:01 +1000 (EST) Received: (qmail 17109 invoked by alias); 5 Sep 2011 17:07:58 -0000 Received: (qmail 17100 invoked by uid 22791); 5 Sep 2011 17:07:57 -0000 X-SWARE-Spam-Status: No, hits=-0.1 required=5.0 tests=AWL, BAYES_50, MISSING_HEADERS, RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 05 Sep 2011 17:07:40 +0000 Received: (qmail 5319 invoked from network); 5 Sep 2011 17:07:38 -0000 Received: from unknown (HELO ?192.168.0.104?) (ams@127.0.0.2) by mail.codesourcery.com with ESMTPA; 5 Sep 2011 17:07:38 -0000 Message-ID: <4E6501D5.6030002@codesourcery.com> Date: Mon, 05 Sep 2011 18:07:33 +0100 From: Andrew Stubbs User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:6.0.1) Gecko/20110830 Thunderbird/6.0.1 MIME-Version: 1.0 CC: Jakub Jelinek , "Joseph S. Myers" , gcc-patches@gcc.gnu.org, patches@linaro.org Subject: Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant) References: <4E5F6B5F.2020207@codesourcery.com> <4E5FA41F.9010201@codesourcery.com> <4E5FA4D5.9050309@codesourcery.com> <20110901155136.GS2687@tyan-ft48-01.lab.bos.redhat.com> <4E5FB10B.7060005@codesourcery.com> In-Reply-To: <4E5FB10B.7060005@codesourcery.com> Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On 01/09/11 17:21, Andrew Stubbs wrote: > I wasn't sure how to find the mode of shift operand in the predicate > though, so I've assumed they're always the same size. How would one find > the proper mode in a predicate? OK, no reply, so I'm just going to assume we're dealing with 32-bit registers. Additionally, Richard Sandiford has pointed out that changing the predicate such that it is more restrictive than the constraints is a problem because reload apparently ignores the predicates under certain circumstances. Setting aside that that seems broken and wrong (what's the point in a predicate if you're just going to ignore it), this patch also creates a new constraint "Pm" that limits the range to match the predicate. Speaking of which, I've limited the constants to the range 1..31 (inclusive) because a) allowing zero seems like it would be counter-productive - it would be better to keep a zero shift as a separate operation that can be optimized away (probably not an issue, but there it is); and b) allowing zero would produce non-canonical assembler (which is not a problem now, but is still best avoided). I have a bootstrap test running now. Assuming that succeeds, is this ok? Andrew 2011-09-05 Andrew Stubbs gcc/ * config/arm/arm.md (*not_shiftsi, *not_shiftsi_compare0, *not_shiftsi_compare0_scratch, *cmpsi_shiftsi, *cmpsi_shiftsi_swp, *arith_shiftsi, *arith_shiftsi_compare0, *arith_shiftsi_compare0_scratch, *sub_shiftsi, *sub_shiftsi_compare0, *sub_shiftsi_compare0_scratch): Change 'M' constraint to 'Pm'. * config/arm/constraints.md (Pm): New constraint. * config/arm/predicates.md (shift_amount_operand): Ensure that all constants satisfy the Pm constraint. gcc/testsuite/ * testsuite/gcc.dg/pr50193-1.c: New file. --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -3669,7 +3669,7 @@ [(set (match_operand:SI 0 "s_register_operand" "=r,r") (not:SI (match_operator:SI 3 "shift_operator" [(match_operand:SI 1 "s_register_operand" "r,r") - (match_operand:SI 2 "shift_amount_operand" "M,rM")])))] + (match_operand:SI 2 "shift_amount_operand" "Pm,rPm")])))] "TARGET_32BIT" "mvn%?\\t%0, %1%S3" [(set_attr "predicable" "yes") @@ -3683,7 +3683,7 @@ (compare:CC_NOOV (not:SI (match_operator:SI 3 "shift_operator" [(match_operand:SI 1 "s_register_operand" "r,r") - (match_operand:SI 2 "shift_amount_operand" "M,rM")])) + (match_operand:SI 2 "shift_amount_operand" "Pm,rPm")])) (const_int 0))) (set (match_operand:SI 0 "s_register_operand" "=r,r") (not:SI (match_op_dup 3 [(match_dup 1) (match_dup 2)])))] @@ -3700,7 +3700,7 @@ (compare:CC_NOOV (not:SI (match_operator:SI 3 "shift_operator" [(match_operand:SI 1 "s_register_operand" "r,r") - (match_operand:SI 2 "shift_amount_operand" "M,rM")])) + (match_operand:SI 2 "shift_amount_operand" "Pm,rPm")])) (const_int 0))) (clobber (match_scratch:SI 0 "=r,r"))] "TARGET_32BIT" @@ -7247,7 +7247,7 @@ (compare:CC (match_operand:SI 0 "s_register_operand" "r,r") (match_operator:SI 3 "shift_operator" [(match_operand:SI 1 "s_register_operand" "r,r") - (match_operand:SI 2 "shift_amount_operand" "M,rM")])))] + (match_operand:SI 2 "shift_amount_operand" "Pm,rPm")])))] "TARGET_32BIT" "cmp%?\\t%0, %1%S3" [(set_attr "conds" "set") @@ -7259,7 +7259,7 @@ [(set (reg:CC_SWP CC_REGNUM) (compare:CC_SWP (match_operator:SI 3 "shift_operator" [(match_operand:SI 1 "s_register_operand" "r,r") - (match_operand:SI 2 "shift_amount_operand" "M,rM")]) + (match_operand:SI 2 "shift_amount_operand" "Pm,rPm")]) (match_operand:SI 0 "s_register_operand" "r,r")))] "TARGET_32BIT" "cmp%?\\t%0, %1%S3" @@ -8649,7 +8649,7 @@ (match_operator:SI 1 "shiftable_operator" [(match_operator:SI 3 "shift_operator" [(match_operand:SI 4 "s_register_operand" "r,r,r,r") - (match_operand:SI 5 "shift_amount_operand" "M,M,M,r")]) + (match_operand:SI 5 "shift_amount_operand" "Pm,Pm,Pm,r")]) (match_operand:SI 2 "s_register_operand" "rk,rk,r,rk")]))] "TARGET_32BIT" "%i1%?\\t%0, %2, %4%S3" @@ -8700,7 +8700,7 @@ (match_operator:SI 1 "shiftable_operator" [(match_operator:SI 3 "shift_operator" [(match_operand:SI 4 "s_register_operand" "r,r") - (match_operand:SI 5 "shift_amount_operand" "M,r")]) + (match_operand:SI 5 "shift_amount_operand" "Pm,r")]) (match_operand:SI 2 "s_register_operand" "r,r")]) (const_int 0))) (set (match_operand:SI 0 "s_register_operand" "=r,r") @@ -8719,7 +8719,7 @@ (match_operator:SI 1 "shiftable_operator" [(match_operator:SI 3 "shift_operator" [(match_operand:SI 4 "s_register_operand" "r,r") - (match_operand:SI 5 "shift_amount_operand" "M,r")]) + (match_operand:SI 5 "shift_amount_operand" "Pm,r")]) (match_operand:SI 2 "s_register_operand" "r,r")]) (const_int 0))) (clobber (match_scratch:SI 0 "=r,r"))] @@ -8735,7 +8735,7 @@ (minus:SI (match_operand:SI 1 "s_register_operand" "r,r") (match_operator:SI 2 "shift_operator" [(match_operand:SI 3 "s_register_operand" "r,r") - (match_operand:SI 4 "shift_amount_operand" "M,r")])))] + (match_operand:SI 4 "shift_amount_operand" "Pm,r")])))] "TARGET_32BIT" "sub%?\\t%0, %1, %3%S2" [(set_attr "predicable" "yes") @@ -8749,7 +8749,7 @@ (minus:SI (match_operand:SI 1 "s_register_operand" "r,r") (match_operator:SI 2 "shift_operator" [(match_operand:SI 3 "s_register_operand" "r,r") - (match_operand:SI 4 "shift_amount_operand" "M,rM")])) + (match_operand:SI 4 "shift_amount_operand" "Pm,rPm")])) (const_int 0))) (set (match_operand:SI 0 "s_register_operand" "=r,r") (minus:SI (match_dup 1) @@ -8767,7 +8767,7 @@ (minus:SI (match_operand:SI 1 "s_register_operand" "r,r") (match_operator:SI 2 "shift_operator" [(match_operand:SI 3 "s_register_operand" "r,r") - (match_operand:SI 4 "shift_amount_operand" "M,rM")])) + (match_operand:SI 4 "shift_amount_operand" "Pm,rPm")])) (const_int 0))) (clobber (match_scratch:SI 0 "=r,r"))] "TARGET_32BIT" --- a/gcc/config/arm/constraints.md +++ b/gcc/config/arm/constraints.md @@ -31,7 +31,7 @@ ;; The following multi-letter normal constraints have been used: ;; in ARM/Thumb-2 state: Da, Db, Dc, Dn, Dl, DL, Dv, Dy, Di, Dz ;; in Thumb-1 state: Pa, Pb, Pc, Pd -;; in Thumb-2 state: Pj, PJ, Ps, Pt, Pu, Pv, Pw, Px, Py +;; in Thumb-2 state: Pj, PJ, Pm, Ps, Pt, Pu, Pv, Pw, Px, Py ;; The following memory constraints have been used: ;; in ARM/Thumb-2 state: Q, Ut, Uv, Uy, Un, Um, Us @@ -172,6 +172,12 @@ (and (match_code "const_int") (match_test "TARGET_THUMB1 && ival >= 0 && ival <= 7"))) +(define_constraint "Pm" + "@internal In ARM/Thumb2 state, a constant in the range 1 to 31" + (and (match_code "const_int") + (match_test "TARGET_32BIT + && IN_RANGE (ival, 1, 31)"))) + (define_constraint "Ps" "@internal In Thumb-2 state a constant in the range -255 to +255" (and (match_code "const_int") --- a/gcc/config/arm/predicates.md +++ b/gcc/config/arm/predicates.md @@ -132,7 +132,8 @@ (define_predicate "shift_amount_operand" (ior (and (match_test "TARGET_ARM") (match_operand 0 "s_register_operand")) - (match_operand 0 "const_int_operand"))) + (and (match_operand 0 "const_int_operand") + (match_test "satisfies_constraint_Pm (op)")))) (define_predicate "arm_add_operand" (ior (match_operand 0 "arm_rhs_operand") --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr50193-1.c @@ -0,0 +1,10 @@ +/* PR 50193: ARM: ICE on a | (b << negative-constant) */ +/* Ensure that the compiler doesn't ICE. */ + +/* { dg-options "-O2" } */ + +int +foo(int a, int b) +{ + return a | (b << -3); /* { dg-warning "left shift count is negative" } */ +}