From patchwork Mon Apr 22 02:15:08 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dehao Chen X-Patchwork-Id: 238284 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 3583B2C04BD for ; Mon, 22 Apr 2013 12:15:18 +1000 (EST) 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:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=EG5aYstEWHGUMYKV7B Cq4zRTyb7OZrgqI+8bKhr91VuMvC5vh9tR8E1AbYR53GPiw/7ho9nVPPoWZNWvG5 3QUDHh5pVWLHsM0MvZhpbZ7AFa7UgY2uYK/ToA7x465P5qVA+Z92sSmXXvBQXP1/ 67VUsHyhMFrnLtA9pdVx6zx5w= 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:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=8XrEUMx2vokYrNplV5cmOZD0 KIE=; b=dFPmX8pzmbMc0dpPcrnVRmmRpYr7cr2XTHUJI+bAgk4ql6RvpcZCFut2 6QlZ/VG4wq8cspGm9zY7DtgOur89ZFBMu297O4WluDJGb9S7C9HxQue7Y6j02+Ld kvxdRJHmfkasRH49sOrcNUFQpHUAmLvbXjqpf6mDF9jE5K022ho= Received: (qmail 16247 invoked by alias); 22 Apr 2013 02:15:12 -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 16237 invoked by uid 89); 22 Apr 2013 02:15:11 -0000 X-Spam-SWARE-Status: No, score=-5.3 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_TRUST, KHOP_THREADED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE, RP_MATCHES_RCVD autolearn=ham version=3.3.1 Received: from mail-wg0-f51.google.com (HELO mail-wg0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 22 Apr 2013 02:15:10 +0000 Received: by mail-wg0-f51.google.com with SMTP id b12so559358wgh.18 for ; Sun, 21 Apr 2013 19:15:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type:x-gm-message-state; bh=PBF+clZ7beC9F+iPeqVpZXjWGIQ2+APpcCTbmMdNWqQ=; b=gMvuYYJeIPh2G5WKpuYe7GQCWrAjCvU9IXDawQIewFoNsGTqDgO8tPOoBASsiYpFNO C1OWo6puWvgL3jtq9HTnwn2urYnsv490ey+0TVWg63LXMaS08UM6f6GWz6OcLr8EVOgz xQ8EWhS7qIMc9pZkCnoPA25dZmWOwhOXd0K0NEWNT+7txayVlRaVzSGXSLtTj6p1OjDE IP413kcI+AF9SAEklvdSj2RYWUJUQ9Qkb4Rrli0ruVGvlXhS8PVwRuVEJYmxj8bjIZ+K 0XrbGVTOc43sapOuWbJJpS8nJUMPq8GMxqQYsyO14YUDFMmOZjrVohgExi3xFS42VEvF VKIg== MIME-Version: 1.0 X-Received: by 10.194.122.131 with SMTP id ls3mr21476306wjb.55.1366596908206; Sun, 21 Apr 2013 19:15:08 -0700 (PDT) Received: by 10.14.214.195 with HTTP; Sun, 21 Apr 2013 19:15:08 -0700 (PDT) In-Reply-To: References: Date: Sun, 21 Apr 2013 19:15:08 -0700 Message-ID: Subject: Re: [GOOGLE] Workaround a bug in AutoFDO which may leads to infinite loop for inlined assembly From: Dehao Chen To: Teresa Johnson Cc: GCC Patches , David Li X-Gm-Message-State: ALoCoQl6OvBghoEgxme5Vsmh1NUVPdWGFze89sne5SSaWfxRUw03VUlXs36hmdyz1/X9ax3YmqkbGTh6TOrU1YkmjketCU+QVBX7zjQVjMh7H0fQurO2/utLtTRrXdiWNZ7gDOIWJm6B2maiAgLBrQdbteP76qBzi9+Qioq5mP1Bktt2Hj0n6o8UElj4rBE2C1B65HECeHGk Thanks for the comment, new patch attached: On Sun, Apr 21, 2013 at 5:00 PM, Teresa Johnson wrote: > On Sun, Apr 21, 2013 at 4:51 PM, Dehao Chen wrote: >> This patch fixed a bug in getting inline stacks: if there is no >> location info attached to a block, we should *not* try to get its >> function name because it could result in infinite loop. > > Is the infinite loop within get_function_decl_from_block itself? If > so, for extra safety perhaps that routine should check if the location > is unknown and return early if it is. One other minor comment below, > otherwise it looks ok to me. > > Teresa > >> >> Bootstrapped and passed all regression tests. >> >> Ok for gcc-4_7 branch? >> >> Thanks, >> Dehao >> >> Index: gcc/auto-profile.c >> =================================================================== >> --- gcc/auto-profile.c (revision 198117) >> +++ gcc/auto-profile.c (working copy) >> @@ -662,9 +662,10 @@ get_inline_stack_by_stmt (gimple stmt, tree decl, >> block && (TREE_CODE (block) == BLOCK); >> block = BLOCK_SUPERCONTEXT (block)) >> { >> - tree decl = get_function_decl_from_block (block); >> + tree decl; >> if (LOCATION_LOCUS (BLOCK_SOURCE_LOCATION (block)) == UNKNOWN_LOCATION) >> continue; >> + decl = get_function_decl_from_block (block); >> loc = BLOCK_SOURCE_LOCATION (block); > > Do you want to move up the assignment to loc and use loc in the new if > statement? > >> pos_stack[idx].file = expand_location (loc).file; >> pos_stack[idx].line = expand_location (loc).line; > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 Index: gcc/auto-profile.c =================================================================== --- gcc/auto-profile.c (revision 198117) +++ gcc/auto-profile.c (working copy) @@ -623,6 +623,10 @@ static tree get_function_decl_from_block (tree block) { tree decl; + + if (LOCATION_LOCUS (BLOCK_SOURCE_LOCATION (block) == UNKNOWN_LOCATION)) + return NULL_TREE; + for (decl = BLOCK_ABSTRACT_ORIGIN (block); decl && (TREE_CODE (decl) == BLOCK); decl = BLOCK_ABSTRACT_ORIGIN (decl)) @@ -662,10 +666,12 @@ get_inline_stack_by_stmt (gimple stmt, tree decl, block && (TREE_CODE (block) == BLOCK); block = BLOCK_SUPERCONTEXT (block)) { - tree decl = get_function_decl_from_block (block); - if (LOCATION_LOCUS (BLOCK_SOURCE_LOCATION (block)) == UNKNOWN_LOCATION) + tree decl; + loc = BLOCK_SOURCE_LOCATION (block); + + if (LOCATION_LOCUS (loc) == UNKNOWN_LOCATION) continue; - loc = BLOCK_SOURCE_LOCATION (block); + decl = get_function_decl_from_block (block); pos_stack[idx].file = expand_location (loc).file; pos_stack[idx].line = expand_location (loc).line; pos_stack[idx - 1].func =