From patchwork Fri Nov 4 02:48:31 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Stump X-Patchwork-Id: 691125 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3t95rh3F0zz9t1P for ; Fri, 4 Nov 2016 13:49:07 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="kID2BQoQ"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; q=dns; s= default; b=sJEelkRgUNRUQVoz5Dmxep82brILCNtkE6CS2grw2MoZryIOW/KcW AknL/dloO44WXq2QIOuu5P3AFT9wmsHJC0QbuzrKNzYyPUtiPzj004Yn4KIasy02 4Z6TWrZ+NsBeTQslVpAGDsmcSavTzFqK/XMUFcUxfTb4q9OGypAm4c= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; s=default; bh=l6fKFZzhLTlmhRqHTW8LxCcq8RM=; b=kID2BQoQdEKUuPib1jcd4Qx0gUfh HJUEk2cZGjFohF9W1oofIoItXCviU4JIpHRX1/cvyn+OYJ1MQHPQuYo4E9vRrrQC NLkVrvBU0JOJUDDRGcAg8xbjcxXz3c0e53J/RNVOcSzFN8M55t0UepEO7EwHkfoP cuXktDi+X6xmIFI= Received: (qmail 112444 invoked by alias); 4 Nov 2016 02:48:47 -0000 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 Received: (qmail 112322 invoked by uid 89); 4 Nov 2016 02:48:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.1 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=subreg, mixture, SUBREG, Pmode X-HELO: resqmta-po-04v.sys.comcast.net Received: from resqmta-po-04v.sys.comcast.net (HELO resqmta-po-04v.sys.comcast.net) (96.114.154.163) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 04 Nov 2016 02:48:35 +0000 Received: from resomta-po-06v.sys.comcast.net ([96.114.154.230]) by resqmta-po-04v.sys.comcast.net with SMTP id 2UYTcT6paGkXB2UYTc0rms; Fri, 04 Nov 2016 02:48:33 +0000 Received: from up.mrs.kithrup.com ([24.4.193.248]) by resomta-po-06v.sys.comcast.net with SMTP id 2UYScic86GdiR2UYScdkW0; Fri, 04 Nov 2016 02:48:33 +0000 Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\)) Subject: Re: [patch, testsuite] Support dg-require-effective-target label_offsets. From: Mike Stump In-Reply-To: <197a4e63-9118-b600-f4f7-66047952ba30@gjlay.de> Date: Thu, 3 Nov 2016 19:48:31 -0700 Cc: Bernd Schmidt , gcc-patches Message-Id: References: <7312dc33-1570-05cb-d532-a55f271d9b3c@redhat.com> <711074f3-d9df-78c1-3981-303b18ed43af@gjlay.de> <197a4e63-9118-b600-f4f7-66047952ba30@gjlay.de> To: Georg-Johann Lay X-CMAE-Envelope: MS4wfM+96eEtH6f+B03WMsovrVgLeEKxFdXkSHrli9xn6pM87lGbYeIPK8LUuYjmrqlUi2Hs2Xd7eUSZPV1G3ntOnz8qt6DJbHYC7QJkmUOEEyW9xB1tzmOJ tn+PCKGw8eFESZp58OGMwTXAazC9dtN7cozeaxZkWfhVVsSK+gIXuXY/Czygh3oDSbgNufdP0g4UaosmDbdHj/mk3dELJsF6rQq32wg0Ij3hj07eUYVEy7ec X-IsSubscribed: yes On Nov 3, 2016, at 8:25 AM, Georg-Johann Lay wrote: > > On 28.10.2016 17:58, Mike Stump wrote: >> On Oct 27, 2016, at 3:16 AM, Georg-Johann Lay wrote: >>> >>> Now imagine some arithmetic like &&LAB2 - &&LAB1. This might result in >>> one or two stub addresses, and difference between such addresses is a >>> complete different thing than the difference between the original labels: >>> The result might differ in absolute value and in sign, i.e. you can't even >>> determine whether LAB1 or LAB2 comes first in the generated code as the >>> order of stubs might differ from the order of respective labels. >> >> So, I think this all doesn't matter any. Every address gs(LAB) fits in >> 16-bits by definition, and every gs(LAB1) - gs(LAB2) fits into 16 bits and > > Yes, you are right. Label differences could be implemented. AFAIK there is currently no activity by the Binutils folks to add respective support, so it is somewhat pointless to add that support to avr-gcc... [ pause, built an avr compiler ] So, I checked code-gen for gcc.c-torture/compile/labels-2.c and it looked fine: ldi r24,lo8(gs(.L2)) ldi r25,hi8(gs(.L2)) std Y+2,r25 std Y+1,r24 ldi r18,lo8(gs(.L3)) ldi r19,hi8(gs(.L3)) ldi r24,lo8(gs(.L4)) ldi r25,hi8(gs(.L4)) mov r20,r18 mov r21,r19 sub r20,r24 So, apparently quite a lot of the code-gen is already wired up. Since this generated code, I wasn't sure what error you're trying to hide? I tried all the test cases your mentioned, none failed to produce code or failed to assemble that code with -mmcu=atmega2560 nor any other cpu I could find, so, trivially, I must not quite understand what doesn't work yet. I did notice that gcc.c-torture/execute/pr70460.c produced: .word .L3-(.L2) which seems wrong, given all the other code-gen. I think this has to be gs(.L3)-(gs(.L2)), no? I tried to get binutils to do that for me directly with a .word and it seemed resistant; which is unfortunate. If you consider the above to be a code-gen bug, then you can do something like: to get the compiler to error out to prevent bad code-gen that binutils can't handle. If you want, you can also declare by fiat that subtraction might be performed on the stub or the actual function, and so any code that does this isn't supported (runtime unspecified behavior), and then you just mark the execute testcase as bad (not supportable on avr, since avr blew the binutils support for gs). The compile only test cases work fine now, so nothing needs to be done for them. If you go for the above and make it a hard error, then a patch in the style of the original patch would be fine, but please just mark the ones that fail and please key of avr*, as this is an avr-only misfeature. If avr did it right, they would complete gs() support so that gs(LAB)-gs(LAB) works. gcc has a fetish for + and - working with labels and constants. The completely different solution would be to never use gs(), and generate code that is 3 byte aware, but, I suspect you're unlikely to want to do that. > Bottom line is that label offsets are not supported by avr BE, and the intention of the patch is to reduce FAIL noise in testsuite results because of the missing support. So, what doesn't work? I tried all the tests cases on a top of tree compiler, and nothing I did ever failed. >> thus is valid for all 16-bit one contexts. The fact the order between the >> stub and the actual code is different is irrelevant, it is a private > > Only if the code is not executed; there are several test cases that compute relative placements of labels like: > > x(){if(&&e-&&b<0)x();b:goto*&&b;e:;} > > If a test ever relies on the fact that &&e - &&b tells anything about the ordering of the labels, the code is about to crash. No, I checked the code, it's perfectly fine, with the one exception that I think .L3 is used in places where gs(.L3) should be used. The single bug I found is a port bug, and it wants to sometimes use gs() and sometimes not use gs(). You can't do that. For the totality of what people want to work, you have to either use gs() everywhere, or nowhere; it is the mixture that is the problem. And then, the only thing that bug prevents is one test case from running. That test case could be skipped on avr, if it doesn't work reliably, as most other port maintainers would rather be consistent, and if they are, they can't hit this avr port bug. Also, the code produced isn't run, so arguing what would happen when run is silly, as, that's not the case in any of the tests except one of them. For the last one, there would appear to be a compiler code-gen bug that needs fixing as mentioned above. Index: config/avr/avr.c =================================================================== --- config/avr/avr.c (revision 241837) +++ config/avr/avr.c (working copy) @@ -9422,6 +9422,21 @@ x = plus_constant (Pmode, x, AVR_TINY_PM_OFFSET); } + /* We generate stubs using gs(), but binutils doesn't support that + here. */ + if (AVR_3_BYTE_PC + && GET_CODE (x) == MINUS + && GET_CODE (XEXP (x, 0)) == LABEL_REF + && GET_CODE (XEXP (x, 1)) == LABEL_REF) + return false; + if (AVR_3_BYTE_PC + && GET_CODE (x) == MINUS + && GET_CODE (XEXP (x, 0)) == SUBREG + && GET_CODE (XEXP (XEXP (x, 0), 0)) == LABEL_REF + && GET_CODE (XEXP (x, 1)) == SUBREG + && GET_CODE (XEXP (XEXP (x, 1), 0)) == LABEL_REF) + return false; + return default_assemble_integer (x, size, aligned_p); }