From patchwork Thu Oct 16 17:17:41 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Lance Taylor X-Patchwork-Id: 400338 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 851521400B5 for ; Fri, 17 Oct 2014 04:17:58 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:mime-version:content-type; q=dns; s=default; b=hOfnZzx9OFhM+e481FCoNu/HK4CBCJ4cfQqnv/Ggz6tfVnnkF3 BQlQHatrt3cyGdG3dpADtrjvB9F+Vo/fUVjRjuzVuH+qhbD9F6SE+ELPo+9lFcST 991C5xtnQb3UioaiSq+cBvm1w8k5Nkx2k6Ja2uO248Wue9/DqFiPezgqo= 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:from :to:cc:subject:date:message-id:mime-version:content-type; s= default; bh=Z/5A4sq8zy6Krl1QgXCES9jMK80=; b=TQrc2I69z31vOPsuMRrT WLGoi5mKoCJJ9nlaFAVj6R0dDFOfnFiifumrblAj5/Dt6W+7VAsuXTiPEVZfITgy /uxIJynYVE+Tq7HxrVBTveVpVCCEbpWmWUt51KBxmwDgENnB1eCv9xa2lEVk01D2 X0iNacjBMhqLIVGxx0Y1rGQ= Received: (qmail 25221 invoked by alias); 16 Oct 2014 17:17:51 -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 25211 invoked by uid 89); 16 Oct 2014 17:17:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-pa0-f52.google.com Received: from mail-pa0-f52.google.com (HELO mail-pa0-f52.google.com) (209.85.220.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 16 Oct 2014 17:17:46 +0000 Received: by mail-pa0-f52.google.com with SMTP id fb1so3775756pad.25 for ; Thu, 16 Oct 2014 10:17:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-type; bh=E1rnoCBEOZ46VL/arRTv72+r72t1yRn32jlGGd2zWPU=; b=EegMFv60x2thEbII34jWlcJqzF3QxChIS9Fao4ukCEwLRrq3XAURyKeBiBWt0AnVOn 4atSO8tbV3ZrmHQcmBDjUXO48uQDPjYYQKVjRpBSLmxc42d9CST+gFnqFvdX9ibux97D OmpxTh0t74DI3X6DmuucDnslc4JA3oUcRyaPCL7gO3/GNTZG2YCVEnD2H6ZdqyW0kB4J JBBKG3JKDZHkHOPVXXVfTu7IZzm8iXe1cyLoVUOpa2YQaMHZUenmXY+Cgq8qt93E0wNB ZMK0R9wPFqvPCZg6FDVyeOFyO4F3MsMo1WPigKrFvJzOkVIJn+dHsZZNWRjaWVfeWxA3 W4hQ== X-Gm-Message-State: ALoCoQkw7WNXn+tSz2qlfIvNN/2GNTS6cqc8mmmxNsLsUbG6DPRUtf1Z9uVQtbtPqtJ33YZSD0aD X-Received: by 10.66.235.136 with SMTP id um8mr2595053pac.143.1413479865016; Thu, 16 Oct 2014 10:17:45 -0700 (PDT) Received: from iant-glaptop.roam.corp.google.com.google.com ([172.26.53.27]) by mx.google.com with ESMTPSA id go1sm20190485pbd.77.2014.10.16.10.17.43 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 16 Oct 2014 10:17:43 -0700 (PDT) From: Ian Lance Taylor To: gcc-patches@gcc.gnu.org, gofrontend-dev@googlegroups.com Cc: Martin Liska , Jan Hubicka Subject: Go patch committed: Functions that call defer_retaddr not inlinable Date: Thu, 16 Oct 2014 10:17:41 -0700 Message-ID: MIME-Version: 1.0 X-IsSubscribed: yes It does not work to split functions that call __go_set_defer_retaddr, because those functions are designed to work as a single unit. They always look like this: if (__go_set_defer_retaddr (&&L)) goto L; real_function (real_args); L: The function __go_set_defer_retaddr always returns false, but the middle-end doesn't know that. Unfortunately this look like a candidate for splitting, but it really isn't because we want the label to appear immediately after the all to the real function, so that the real function can use it to determine whether is permitted to call recover. In general nothing goes wrong, because the function is split and then recombined. But we shouldn't let it be split in the first place. This is PR 63560, and the proposed solution is to not split functions marked as not inlinable. The point of splitting a function is so that the split off part can be inlined, but that is inappropriate if the function is marked non-inlinable. This patch implements this on the Go side by marking these functions as non-inlinable. This has no effect other than on splitting optimizations, as these functions are never called directly; they are only ever passed as function pointers to __go_defer, so there is never an inlining opportunity in any case. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Ian diff -r 551d7176fbd7 go/gogo.cc --- a/go/gogo.cc Mon Oct 13 14:02:46 2014 -0700 +++ b/go/gogo.cc Thu Oct 16 10:07:52 2014 -0700 @@ -4945,6 +4945,14 @@ // our return address comparison. bool is_inlinable = !(this->calls_recover_ || this->is_recover_thunk_); + // If a function calls __go_set_defer_retaddr, then mark it as + // uninlinable. This prevents the GCC backend from splitting + // the function; splitting the function is a bad idea because we + // want the return address label to be in the same function as + // the call. + if (this->calls_defer_retaddr_) + is_inlinable = false; + // If this is a thunk created to call a function which calls // the predeclared recover function, we need to disable // stack splitting for the thunk. diff -r 551d7176fbd7 go/gogo.h --- a/go/gogo.h Mon Oct 13 14:02:46 2014 -0700 +++ b/go/gogo.h Thu Oct 16 10:07:52 2014 -0700 @@ -1065,6 +1065,12 @@ set_has_recover_thunk() { this->has_recover_thunk_ = true; } + // Record that this function is a thunk created for a defer + // statement that calls the __go_set_defer_retaddr runtime function. + void + set_calls_defer_retaddr() + { this->calls_defer_retaddr_ = true; } + // Mark the function as going into a unique section. void set_in_unique_section() @@ -1190,6 +1196,9 @@ bool is_recover_thunk_ : 1; // True if this function already has a recover thunk. bool has_recover_thunk_ : 1; + // True if this is a thunk built for a defer statement that calls + // the __go_set_defer_retaddr runtime function. + bool calls_defer_retaddr_ : 1; // True if this function should be put in a unique section. This is // turned on for field tracking. bool in_unique_section_ : 1; diff -r 551d7176fbd7 go/statements.cc --- a/go/statements.cc Mon Oct 13 14:02:46 2014 -0700 +++ b/go/statements.cc Thu Oct 16 10:07:52 2014 -0700 @@ -2376,6 +2376,8 @@ location); s->determine_types(); gogo->add_statement(s); + + function->func_value()->set_calls_defer_retaddr(); } // Get a reference to the parameter.