From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 124578 invoked by alias); 8 May 2017 15:37:43 -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 124568 invoked by uid 89); 8 May 2017 15:37:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=H*r:10.200.3 X-HELO: mail-qk0-f176.google.com Received: from mail-qk0-f176.google.com (HELO mail-qk0-f176.google.com) (209.85.220.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 08 May 2017 15:37:41 +0000 Received: by mail-qk0-f176.google.com with SMTP id y201so22707964qka.0 for ; Mon, 08 May 2017 08:37:43 -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:cc; bh=UVo6SjanaNn0hXmwVUJDHDiTsTZHlc56sFY7alB1adw=; b=CVuKsqjt1Yept4OtMjg0DhBklYlV5W3J3s2+kzdhuAMJnOEPlyFKJoE7RZz5Y4q9X0 MLPMbYDRlxRfXck+Z9hsqHTEmmol/iUJR0UsbfENBYD2S+JYIUtqymkk7f0lL8YToY4Y E4StDyHlGDg/00iqPexFi/PNPR8gJKpZtsp+Tlge3gn7du6DL5opQcn2+e71tjL66Vn5 gSZ/9L9jQaExaqfQBBQ6IlRBXpHuXGS53ykxEoO0s14+d/wduuO/3v+j1Grlu8Z3D1fT eOpu9cixXWKTBT5JI2ApjwrgcLMaCB01R+ViLLBK1IIOahG6b0p3LImkpwlPcz1zYgbM KMMA== X-Gm-Message-State: AODbwcC15/kCKj3uenJ44QPzudEtEsr0YonZHcF4cQfa+bgN1zmOCCmj En2ec/nBoqLoq2ydsG2uQPS4C0OGBw== X-Received: by 10.55.129.66 with SMTP id c63mr16563448qkd.183.1494257862051; Mon, 08 May 2017 08:37:42 -0700 (PDT) MIME-Version: 1.0 Received: by 10.200.3.38 with HTTP; Mon, 8 May 2017 08:37:01 -0700 (PDT) From: Alexander Ivchenko Date: Mon, 08 May 2017 15:50:00 -0000 Message-ID: Subject: [CHKP] Attempt to fix PR79765 (multiversioning and instrumentation) To: Ilya Enkovich Cc: GCC Patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00559.txt.bz2 Hi, I'm trying to fix the problem with function multiversioning and MPX instrumentation (PR79765) and I face several issues. I would appreciate your advice: The first problem that arises is that multiversioning tries to make versions out of thunks, which do not have bodies. This is fixed with this patch: diff --git a/gcc/ipa-chkp.c b/gcc/ipa-chkp.c index c7fc3a0..5de11b3 100644 --- a/gcc/ipa-chkp.c +++ b/gcc/ipa-chkp.c @@ -718,6 +718,9 @@ chkp_produce_thunks (bool early) 0, CGRAPH_FREQ_BASE); node->create_reference (node->instrumented_version, IPA_REF_CHKP, NULL); + DECL_ATTRIBUTES (node->decl) + = remove_attribute ("target_clones", + DECL_ATTRIBUTES (node->decl)); /* Thunk shouldn't be a cdtor. */ DECL_STATIC_CONSTRUCTOR (node->decl) = 0; DECL_STATIC_DESTRUCTOR (node->decl) = 0; However, that is not all that is needed to be fixed. Consider case: __attribute__((target_clones("arch=core-avx2", "default"))) int test_fun() { return test_fun(); } Now, in multiple_target.c: create_dispatcher_calls we are replacing the call to test_fun with a call to the ifunc resolvler: (rr) p debug_function (node->decl, TDF_VOPS) __attribute__((target ("default"), target_clones ("arch=core-avx2", "default"))) test_fun () { [100.00%]: _2 = test_fun (); # VUSE <.MEM_1(D)> return _2; } (rr) p e $103 = (cgraph_edge *) 0x7f658d121340 (rr) p e->callee $104 = (rr) p e->caller $105 = (rr) p inode $106 = We do.. e->redirect_callee (inode); e->redirect_call_stmt_to_callee (); And after we have: (rr) p debug_function (node->decl, TDF_VOPS) __attribute__((target ("default"), target_clones ("arch=core-avx2", "default"))) test_fun () { [100.00%]: # .MEM = VDEF <.MEM> _2 = test_fun.ifunc (); # VUSE <.MEM_1(D)> return _2; } For MPX-instrumented code the outcome of the same procedure is different: (rr) p e->caller $5 = (rr) p e->callee $6 = (rr) p inode $7 = (rr) p debug_function (node->decl, TDF_VOPS) __attribute__((target ("default"), chkp instrumented, target_clones ("arch=core-avx2", "default"))) test_fun.chkp () { [100.00%]: _2 = test_fun.chkp (); # VUSE <.MEM_1(D)> return _2; } -- We don't have # .MEM = VDEF <.MEM> part and hence, we hit internal compiler error in verify_ssa, which fails. The reason for that is that in redirect_call_stmt_to_callee for instrumented code the path is different and update_stmt_fn is not called: if (e->callee->clone.combined_args_to_skip || skip_bounds) {..} else { .. update_stmt_fn (DECL_STRUCT_FUNCTION (e->caller->decl), new_stmt); } Calling this function after if-then-else fixes the issue: diff --git a/gcc/cgraph.c b/gcc/cgraph.c index e505b10..3dde20d 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1527,8 +1527,8 @@ cgraph_edge::redirect_call_stmt_to_callee (void) { new_stmt = e->call_stmt; gimple_call_set_fndecl (new_stmt, e->callee->decl); - update_stmt_fn (DECL_STRUCT_FUNCTION (e->caller->decl), new_stmt); } + update_stmt_fn (DECL_STRUCT_FUNCTION (e->caller->decl), new_stmt); /* If changing the call to __cxa_pure_virtual or similar noreturn function, adjust gimple_call_fntype too. */ This call makes sense to me, but is it a correct thing to do? And finally, the third point. In the same if-then-else, in the "if" part there is a call to "gsi_replace", which uses "cfun", but targetclone is ipa pass and hence has cfun defined to NULL. So, is it a bug at all to use "gsi_replace" in ipa pass? thanks in advance, Alexander