From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 96862 invoked by alias); 9 Sep 2016 23:12:06 -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 96822 invoked by uid 89); 9 Sep 2016 23:12:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Sweet, selftesting, self-testing, suspected X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Sep 2016 23:12:02 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6C038C05091E; Fri, 9 Sep 2016 23:12:00 +0000 (UTC) Received: from localhost.localdomain (ovpn-116-111.phx2.redhat.com [10.3.116.111]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u89NBxeD012193; Fri, 9 Sep 2016 19:12:00 -0400 Subject: Re: [PATCH 9/9] cse.c selftests To: Bernd Edlinger , David Malcolm References: Cc: GCC Patches From: Jeff Law Message-ID: <00753d74-ccf6-3074-1934-485f6296ba6c@redhat.com> Date: Fri, 09 Sep 2016 23:17:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-09/txt/msg00563.txt.bz2 On 09/09/2016 07:28 AM, Bernd Edlinger wrote: > Hi David, > >> I attempted to create a reproducer for PR 71779; however I'm not yet >> able to replicate the bogus cse reported there via the test case. > > Thanks, this is just awesome. I immediately had to try your patch. I'd pointed David at 71779 because it was such a good example of the kind of thing we'd like to be able to avoid by using the self-testing framework. ie, someone says "hey, I don't think this code is needed anymore, let's remove it" and hope nothing breaks. We do have the regression suite now which helps considerably, but changes early in the optimization pipeline may easily mask problems introduced much later in the pipeline. With David's work, the goal is to be able to hand off reasonable hunks of RTL to internal routines and see how they manipulate the RTL and verify correctness. Essentially bypassing everything except the routines in question that we want to test. There's a lot of unknowns in this space and I'm hoping that David's work will shed some light on how we can build a robust RTL test framework. > > The main reason for PR 71779 was that this > > (insn 1047 1046 1048 (set (reg:DI 481) > (subreg:DI (reg/f:SI 479) 0)) y.c:12702 -1 > (nil)) > > got transformed into that: > > (insn 1047 1046 1048 107 (set (reg/f:DI 481) > (subreg:DI (reg/f:SI 477) 0)) y.c:12702 50 {*movdi_aarch64} > (expr_list:REG_DEAD (reg/f:SI 479) > (nil))) > > Note the "/f at reg 481. This together with the fact that -mabi=ilp32 > makes Pmode != ptr_mode caused a cascade of different errors. > > For the wrong transformation in combine to happen, we need > Pmode = DImode, ptr_mode=SImode, and REG_POINTER(x) > is true, but only REG_POINTER is really wrong. > > First the good news. The unit test does actually work, but not only > on aarch even without -mabi=ilp32, but also on i386. All that's > missing is a check that we don't get a reg/f here: Sweet. This starts to touch on some of those unanswered questions -- while 71779 was visibly only failing on aarch64, can we build tests which are ultimately target independent? I suspected that would be the case for 71779, but whether or not that will hold in general remains to be seen. > > test_pr71779 () > { > /* Only run this tests for target==aarch64. */ > #ifndef GCC_AARCH64_H > #ifndef I386_OPTS_H > return; > #endif > #endif > ... > tem = cse_main (get_insns (), max_reg_num ()); > ASSERT_EQ (0, tem); > ASSERT_FALSE (REG_POINTER (SET_DEST (PATTERN (get_insn_by_uid (1047))))); > } > > > One minor thing, an unrelated test did fail before the cse test got executed on my aarch64, > so I just commented that part out: > > > /home/ed/gnu/gcc-build-aarch64/./gcc/xgcc -B/home/ed/gnu/gcc-build-aarch64/./gcc/ -xc -S -c /dev/null -fself-test > ../../gcc-trunk/gcc/read-rtl-function.c:923: test_loading_dump_fragment_2: FAIL: ASSERT_TRUE ((((lhs)->frame_related))) > In function 'test_1': > cc1: internal compiler error: in fail, at selftest.c:46 > 0x11be8ae selftest::fail(selftest::location const&, char const*) > ../../gcc-trunk/gcc/selftest.c:46 > 0x115577a test_loading_dump_fragment_2 > ../../gcc-trunk/gcc/read-rtl-function.c:923 > 0x1157cce selftest::read_rtl_function_c_tests() > ../../gcc-trunk/gcc/read-rtl-function.c:1183 > 0x1163d14 selftest::run_tests() > ../../gcc-trunk/gcc/selftest-run-tests.c:66 > 0xb36262 toplev::run_self_tests() > ../../gcc-trunk/gcc/toplev.c:2074 > > > the aarch64 was configured this way: > > ../gcc-trunk/configure --prefix=/home/ed/gnu/aarch64-unknown-elf --target=aarch64-unknown-elf --enable-languages=c --disable-shared --disable-threads --disable-libssp --disable-libgomp --disable-libquadmath --disable-libatomic > > > Maybe one last comment on your patch itself, could you please move > the unit test cases in extra unit test files, or even a self-test tree? That's one of the things we debated for quite a while earlier. For now they're living in the source files which they're testing. Whether or not that's best remains to be seen. jeff