From patchwork Wed Jun 13 20:32:13 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Lance Taylor X-Patchwork-Id: 929081 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-479666-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=golang.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="liGweTXD"; dkim-atps=neutral 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 415dj80dx1z9s0W for ; Thu, 14 Jun 2018 06:32:26 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:content-type; q= dns; s=default; b=P2EtD1tw0waKNCoCQb5pSY/bEFyxaeIIxn2GvfaQJUbCMm H4dBIuxf6HA1VLNXmoelho/IEvp5xeuSXeebkZPWceDdsrmbkbnoCFiX+q2wIvFe FzsoBARdu5/aURr19e+R98FIOivSaCJ7O+64JT5CnzkNFzfkgMNLf0MBUPZfs= 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 :mime-version:from:date:message-id:subject:to:content-type; s= default; bh=iN6QMhnq74aUG4+SxJu4HHF4n5k=; b=liGweTXD0G5yLFwKEoxf OrJziIiAJiITupOXR98iWf7ylNskKjQtTk8+HUOeNFpc+rwIdnCoUCLzDhv29Fsh /1J+CKIcd3Z09qw5hvxYM0Kpbw8Jc+NuwqFw0DYkOB9qgBTwzVS1D58F7yFXMjtT mGHEyao+8c5TZYa6tjvjTE8= Received: (qmail 95735 invoked by alias); 13 Jun 2018 20:32:19 -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 95723 invoked by uid 89); 13 Jun 2018 20:32:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=wind X-HELO: mail-wm0-f43.google.com Received: from mail-wm0-f43.google.com (HELO mail-wm0-f43.google.com) (74.125.82.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 13 Jun 2018 20:32:16 +0000 Received: by mail-wm0-f43.google.com with SMTP id r15-v6so6887705wmc.1 for ; Wed, 13 Jun 2018 13:32:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=qjFSh0QLstmXOuu+hKFgbGBkFqLp9o79Ekly/U9/wmA=; b=JDhzRyjpDB2z1Vz4wQd0HGtK4PvLB8T9W+xCjOuWLekRfAknkzygit8YFe0/710bV5 MduKdjEmK03CgFA9aYIAj7Lo6tDHXejKZE4oMsyLbOq7zEpEiGaiuq7M7iI+FSMEZ3S1 4onezxaBjKNUOjNwRsLk5pn+CgVP+Z9hBVeiZVJOUWsqnXRFrAM1h2nuMYIi/8q5fg/y rlm3iTPfW+sj7Unb2NZsl1XMjsA8mwXiOGIHCiQHnBMN9Xfh4qnKQCwNy2pEjazdjiUx fRFj+uT3vl88XDgLQw0duvsKJcFT2rFB5o0deNX2hWbRzgTrzPlFMWnSmy/w7sZZW0Mc eL5g== X-Gm-Message-State: APt69E1revHIDSRfheioogLcPwSQmmiRzfOYC3dtUYLq/IDJ4jZENcrM kYrVjmT59KN+DMRsRqsQUKPJkOs6KQo5RGqyBUvj4KZi X-Google-Smtp-Source: ADUXVKIfaQwECpcsmuK8CS1JXqcLDRRwdv/+Jr/RlasmwrZj4JasZHaS4KK+H1yYiTsJqwpRHG91N211a2hwq2pYS8U= X-Received: by 2002:aa7:d7cb:: with SMTP id e11-v6mr5687609eds.83.1528921934378; Wed, 13 Jun 2018 13:32:14 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a50:9147:0:0:0:0:0 with HTTP; Wed, 13 Jun 2018 13:32:13 -0700 (PDT) From: Ian Lance Taylor Date: Wed, 13 Jun 2018 13:32:13 -0700 Message-ID: Subject: Go patch committed: Avoid introducing redundant write barriers To: gcc-patches , gofrontend-dev@googlegroups.com This patch to the Go frontend by Than McIntosh fixes the compier to avoid introducing redundant write barriers. The traversal used by the write barrier insertion phase can sometimes wind up visiting new statements inserted during the traversal, which then results in duplicate / redundant write barrier guards. Example program to reproduce: package small type S struct { N *S K int } var G *S = &S{N: nil, K: 101} This patch changes the traversal code to keep track of statements already added and avoid processing them again later in the traversal. This fixes https://golang.org/25867 . Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline. Ian Index: gcc/go/gofrontend/MERGE =================================================================== --- gcc/go/gofrontend/MERGE (revision 261555) +++ gcc/go/gofrontend/MERGE (working copy) @@ -1,4 +1,4 @@ -1f07926263b6d14edb6abd6a00e6385190d30d0e +c3ef5bbf4e4271216b6f22621269d458599e8087 The first line of this file holds the git revision number of the last merge done from the gofrontend repository. Index: gcc/go/gofrontend/gogo.cc =================================================================== --- gcc/go/gofrontend/gogo.cc (revision 261521) +++ gcc/go/gofrontend/gogo.cc (working copy) @@ -8444,6 +8444,9 @@ Traverse::function_declaration(Named_obj void Statement_inserter::insert(Statement* s) { + if (this->statements_added_ != NULL) + this->statements_added_->insert(s); + if (this->block_ != NULL) { go_assert(this->pindex_ != NULL); Index: gcc/go/gofrontend/gogo.h =================================================================== --- gcc/go/gofrontend/gogo.h (revision 261521) +++ gcc/go/gofrontend/gogo.h (working copy) @@ -3419,19 +3419,24 @@ class Traverse class Statement_inserter { public: + typedef Unordered_set(Statement*) Statements; + // Empty constructor. Statement_inserter() - : block_(NULL), pindex_(NULL), gogo_(NULL), var_(NULL) + : block_(NULL), pindex_(NULL), gogo_(NULL), var_(NULL), + statements_added_(NULL) { } // Constructor for a statement in a block. - Statement_inserter(Block* block, size_t *pindex) - : block_(block), pindex_(pindex), gogo_(NULL), var_(NULL) + Statement_inserter(Block* block, size_t *pindex, Statements *added = NULL) + : block_(block), pindex_(pindex), gogo_(NULL), var_(NULL), + statements_added_(added) { } // Constructor for a global variable. - Statement_inserter(Gogo* gogo, Variable* var) - : block_(NULL), pindex_(NULL), gogo_(gogo), var_(var) + Statement_inserter(Gogo* gogo, Variable* var, Statements *added = NULL) + : block_(NULL), pindex_(NULL), gogo_(gogo), var_(var), + statements_added_(added) { go_assert(var->is_global()); } // We use the default copy constructor and assignment operator. @@ -3451,6 +3456,8 @@ class Statement_inserter Gogo* gogo_; // The global variable, when looking at an initializer expression. Variable* var_; + // If non-null, a set to record new statements inserted (non-owned). + Statements* statements_added_; }; // When translating the gogo IR into the backend data structure, this Index: gcc/go/gofrontend/wb.cc =================================================================== --- gcc/go/gofrontend/wb.cc (revision 261521) +++ gcc/go/gofrontend/wb.cc (working copy) @@ -213,7 +213,7 @@ class Write_barriers : public Traverse public: Write_barriers(Gogo* gogo) : Traverse(traverse_functions | traverse_variables | traverse_statements), - gogo_(gogo), function_(NULL) + gogo_(gogo), function_(NULL), statements_added_() { } int @@ -230,6 +230,8 @@ class Write_barriers : public Traverse Gogo* gogo_; // Current function. Function* function_; + // Statements introduced. + Statement_inserter::Statements statements_added_; }; // Traverse a function. Just record it for later. @@ -298,9 +300,10 @@ Write_barriers::variable(Named_object* n Location loc = init->location(); Expression* ref = Expression::make_var_reference(no, loc); - Statement_inserter inserter(this->gogo_, var); + Statement_inserter inserter(this->gogo_, var, &this->statements_added_); Statement* s = this->gogo_->assign_with_write_barrier(NULL, NULL, &inserter, ref, init, loc); + this->statements_added_.insert(s); var->add_preinit_statement(this->gogo_, s); var->clear_init(); @@ -313,6 +316,9 @@ Write_barriers::variable(Named_object* n int Write_barriers::statement(Block* block, size_t* pindex, Statement* s) { + if (this->statements_added_.find(s) != this->statements_added_.end()) + return TRAVERSE_SKIP_COMPONENTS; + switch (s->classification()) { default: @@ -355,7 +361,7 @@ Write_barriers::statement(Block* block, Function* function = this->function_; Location loc = init->location(); - Statement_inserter inserter(block, pindex); + Statement_inserter inserter(block, pindex, &this->statements_added_); // Insert the variable declaration statement with no // initializer, so that the variable exists. @@ -370,6 +376,7 @@ Write_barriers::statement(Block* block, &inserter, ref, init, loc); + this->statements_added_.insert(assign); // Replace the old variable declaration statement with the new // initialization. @@ -391,12 +398,14 @@ Write_barriers::statement(Block* block, // Change the assignment to use a write barrier. Function* function = this->function_; Location loc = as->location(); - Statement_inserter inserter = Statement_inserter(block, pindex); + Statement_inserter inserter = + Statement_inserter(block, pindex, &this->statements_added_); Statement* assign = this->gogo_->assign_with_write_barrier(function, block, &inserter, lhs, rhs, loc); + this->statements_added_.insert(assign); block->replace_statement(*pindex, assign); } break;