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: 3869 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 91E9723EF9 for ; Mon, 5 Sep 2011 17:07:45 +0000 (UTC) Received: from mail-fx0-f52.google.com (mail-fx0-f52.google.com [209.85.161.52]) by fiordland.canonical.com (Postfix) with ESMTP id 7C102A1851D for ; Mon, 5 Sep 2011 17:07:45 +0000 (UTC) Received: by fxd18 with SMTP id 18so5335519fxd.11 for ; Mon, 05 Sep 2011 10:07:45 -0700 (PDT) Received: by 10.223.22.16 with SMTP id l16mr1569329fab.62.1315242465261; Mon, 05 Sep 2011 10:07:45 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.152.11.8 with SMTP id m8cs69796lab; Mon, 5 Sep 2011 10:07:43 -0700 (PDT) Received: by 10.68.5.67 with SMTP id q3mr6910428pbq.4.1315242461757; Mon, 05 Sep 2011 10:07:41 -0700 (PDT) Received: from mail.codesourcery.com (mail.codesourcery.com. [38.113.113.100]) by mx.google.com with ESMTPS id 1si5813874pbp.111.2011.09.05.10.07.40 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 05 Sep 2011 10:07:41 -0700 (PDT) Received-SPF: pass (google.com: domain of ams@codesourcery.com designates 38.113.113.100 as permitted sender) client-ip=38.113.113.100; Authentication-Results: mx.google.com; spf=pass (google.com: domain of ams@codesourcery.com designates 38.113.113.100 as permitted sender) smtp.mail=ams@codesourcery.com 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 Organization: CodeSourcery 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> 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" } */ +}