From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10509 invoked by alias); 22 Sep 2014 11:16:12 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 10497 invoked by uid 89); 22 Sep 2014 11:16:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: service87.mimecast.com Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 22 Sep 2014 11:16:10 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Mon, 22 Sep 2014 12:16:07 +0100 Received: from [10.1.209.51] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 22 Sep 2014 12:16:05 +0100 Message-ID: <542004F5.8080009@arm.com> Date: Mon, 22 Sep 2014 11:16:00 -0000 From: Alan Lawrence User-Agent: Thunderbird 2.0.0.24 (X11/20101213) MIME-Version: 1.0 To: Marcus Shawcroft , Richard Earnshaw CC: "gcc-patches@gcc.gnu.org" , "law@redhat.com" Subject: [AArch64] Re: [PATCH] Relax check against commuting XOR and ASHIFTRT in combine.c References: <53B1B4FE.7010201@arm.com> <53B1D271.5000405@redhat.com> <53C69926.4050503@arm.com> <53C80023.6000100@arm.com> <5409FBB1.3040509@redhat.com> <541AA752.9030302@arm.com> <541BC141.500@redhat.com> In-Reply-To: <541BC141.500@redhat.com> X-MC-Unique: 114092212160705401 Content-Type: text/plain; charset=WINDOWS-1252; format=flowed Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg01802.txt.bz2 Ok thanks Jeff. In that case I think I should draw this to the attention of= the=20 AArch64 maintainers to check the testsuite updates are OK before I commit..= .? Methinks it may be possible to get further, or at least increase our confid= ence,=20 if I "mock" out try_widen_shift_mode, and/or try injecting some dubious RTL= from=20 a builtin, although this'll only give a momentary snapshot of behaviour. I = may=20 or may not have time to look into this though ;)... Cheers, Alan Jeff Law wrote: > On 09/18/14 03:35, Alan Lawrence wrote: >> Moreover, I think we both agree that if result_mode=3D=3Dshift_mode, the >> transformation is correct. "Just" putting that check in, achieves >> what I'm trying for here, so I'd be happy to go with the attached >> patch and call it a day. However, I'm a little concerned about the >> other cases - i.e. where shift_mode is wider than result_mode. > Let's go ahead and get the attached patch installed. I'm pretty sure=20 > it's correct and I know you want to see something move forward here. We= =20 > can iterate further if we want. >=20 >> If I understand correctly (and I'm not sure about that, let's see how >> far I get), this means we'll perform the shift in (say) DImode, when >> we're only really concerned about the lower (say) 32-bits (for an >> originally-SImode shift). > That's the class of cases I'm concerned about. >=20 >=20 > try_widen_shift_mode will in this case >> check that the result of the operation *inside* the shift (in our >> case an XOR) has 33 sign bit copies (via num_sign_bit_copies), i.e. >> that its *top* 32-bits are all equal to the original SImode sign bit. >> of these bits may be fed into the top of the desired SImode >> result by the DImode shift. Right so far? > Correct. >=20 >> AFAICT, num_sign_bit_copies for an XOR, conservatively returns the >> minimum of the num_sign_bit_copies of its two operands. I'm not sure >> whether this is behaviour we should rely on in its callers, or for >> the sake of abstraction we should treat num_sign_bit_copies as a >> black box (which does what it says on the, erm, tin). > Treat it as a black box. It returns the number of known sign bit=20 > copies. There may be more, but never less. >=20 >=20 >> If the former, doesn't having num_sign_bit_copies >=3D the difference >> in size between result_mode and shift_mode, of both operands to the >> XOR, guarantee safety of the commutation (whether the constant is >> positive or negative)? We could perform the shift (in the larger >> mode) on both of the XOR operands safely, then XOR together their >> lower parts. > I had convinced myself that when we flip the sign bit via the XOR and=20 > commute the XOR out that we invalidate the assumptions made when=20 > widening. But I'm not so sure anymore. Damn I hate changes made=20 > without suitable tests :( >=20 > I almost convinced myself the problem is in the adjustment of C2 in the=20 > widened case, but that's not a problem either. At least not on paper. >=20 >> If, however, we want to play safe and ensure that we deal safely with >> any XOR whose top (mode size difference + 1) bits were the same, >> then I think the restriction that the XOR constant is positive is >> neither necessary nor sufficient; rather (mirroring >> try_widen_shift_mode) I think we need that num_sign_bit_copies of the >> constant in shift_mode, is more than the size difference between >> result_mode and shift_mode. > But isn't that the same? Isn't the only case where it isn't the same=20 > when the constant has bits set that are outside the mode of the other=20 > operand? >=20 > Hmm, what about (xor:QI A -1)? In that case -1 will be represented with= =20 > bits outside the precision of QImode. >=20 >> Hmmm. I might try that patch at some point, I think it is the right >> check to make. (Meta-comment: this would be *so*much* easier if we >> could write unit tests more easily!) In the meantime I'd be happy to >> settle for the attached... > No argument on the unit testing comment. It's a major failing in the=20 > design of GCC that we can't easily build a unit testing framework. >=20 > Jeff >=20