From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x731.google.com (mail-qk1-x731.google.com [IPv6:2607:f8b0:4864:20::731]) by sourceware.org (Postfix) with ESMTPS id 2FC123857366 for ; Thu, 2 Jun 2022 10:14:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2FC123857366 Received: by mail-qk1-x731.google.com with SMTP id p197so583756qke.3 for ; Thu, 02 Jun 2022 03:14:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Rq5MexKFJqnIxOr+hExOSwgW/InZbSGu6YQRU1bhe10=; b=Zh8EhLaRI7NhuWeHhZJbpsNUbnSBXhHgUEJGCB6010gnVwFm+VNXkPuYPedyi3VMir /TBmXBxCc6PoaynL55DSk/NuRrlWzIXfF01HUZ9krG3uFPJblnwEq9K4SCXCM+OWotii +QKojR1x7HF7Y5/qTmzZWqzwYoX4BkaFtf3yYZ4ZHjKDWej44RuAEM0GXl8FYcq+JIuP h8T3ZYVYLvEzBLn5h5ULmyQ1s2qsI1DkbthfvMn+1u9q1gEsNyF77pM73dZXXZSV1g8Q tybvkV/DJqIub+O1BHivMTq89tLbDMc9EYiLUtSYU92J1I8DRGYV6iDKL2b5Lx2REk20 pKog== X-Gm-Message-State: AOAM532CyYTP4HFYnDK63msWozvGftqOQxy0OGokg4GRojcB2ieUW6dG IEBOEf/CebBWnoulSBSg3bLfZdb4Mq4OtiVnt+RVV1I7foUSJQ== X-Google-Smtp-Source: ABdhPJybCd6jztFL/6o86pjbUNQEHk1Zgq4OX9N2VDaSNMSfJlg9pEaYkTFrRR5LXsAr0MFJx/e5iMQvPxoazF3NaN8= X-Received: by 2002:a05:620a:2703:b0:6a6:48ba:5175 with SMTP id b3-20020a05620a270300b006a648ba5175mr2745365qkp.350.1654164897603; Thu, 02 Jun 2022 03:14:57 -0700 (PDT) MIME-Version: 1.0 References: <4713782.GXAFRqVoOG@fomalhaut> In-Reply-To: <4713782.GXAFRqVoOG@fomalhaut> From: Richard Biener Date: Thu, 2 Jun 2022 12:14:46 +0200 Message-ID: Subject: Re: [PATCH] Introduce -finstrument-functions-once To: Eric Botcazou Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Jun 2022 10:14:59 -0000 On Tue, May 24, 2022 at 12:49 PM Eric Botcazou via Gcc-patches wrote: > > Hi, > > some time ago we were requested to implement a -finstrument-functions-once > switch in the compiler, with the semantics that the profiling functions be > called only once per instrumented function. The goal was to make it possible > to use it in (large) production binaries to do function-level coverage, so the > overhead must be minimum and, in particular, there is no protection against > data races so the "once" moniker is imprecise. So that also applies to "... and the second profiling function is called before the exit +corresponding to this first entry" specifically "corresponding to this first entry"? As if the second entry exits first will that call the second profiling function or will it really be the thread that called the first profiling function (what happens when that thread terminates before calling the second profiling function? (***)). Consider re-wording this slightly. + /* If -finstrument-functions-once is specified, generate: + + static volatile bool F.0 = true; + bool tmp_first; is there any good reason to make F.0 volatile? That doesn't prevent races. Any reason to make F.0 initialized to true rather than false (bss init?) (***) looking at the implementation the second profiling function can end up being never called when the thread calling the first profiling function does not exit the function. So I wonder if the "optimization"(?) not re-reading F.0 makes sense (it also requires to keep the value of F.0 live across the whole function) Otherwise looks OK to me. Richard. > Tested on x86-64/Linux, OK for the mainline? > > > 2022-05-24 Eric Botcazou > > * common.opt (finstrument-functions): Set explicit value. > (-finstrument-functions-once): New option. > * doc/invoke.texi (Program Instrumentation Options): Document it. > * gimplify.c (build_instrumentation_call): New static function. > (gimplify_function_tree): Invoke it to emit the instrumentation calls > if -finstrument-functions[-once] is specified. > > -- > Eric Botcazou