From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x134.google.com (mail-lf1-x134.google.com [IPv6:2a00:1450:4864:20::134]) by sourceware.org (Postfix) with ESMTPS id D353C3887CED for ; Thu, 8 Dec 2022 10:31:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D353C3887CED Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lf1-x134.google.com with SMTP id p8so1345237lfu.11 for ; Thu, 08 Dec 2022 02:31:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=arHtmW3Y3sxf5IiVl9Nn5JTUi+aSDVKlPLWOsSc++8g=; b=df194AFB/SAZMNPa/ARGxe77Y+ERaOZnaqytAn+QsZyRaGxK9Z+R0qe1REU1pdj4UJ qX+lPkSZ4uLbGB19ctKqLesYCVBPsfPyOp0GKwX2syenly8CttuYh3DCYemp3FvJ/43S lx9gUIfvbM9v+mOLmAy2Qdz5seG3SNOfT3Giq4LFlYhwtgfGSJQ4P/y/a3bbIpmPYvRO 0k+L8cbCFjFNY68G/+9t1WQyhVGLK/PAoOnUj4r6c4vt7XUkwDLDmGfM1RAQHjt9rPwY qJuem6bqvfQX+yzrAjLjV4fQgGtzgyX8+FbTuih0FE2OCMg45VQzITleNodwmqBvDs3W tsMg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=arHtmW3Y3sxf5IiVl9Nn5JTUi+aSDVKlPLWOsSc++8g=; b=IeAUraCHsvJeGup/z7vuCQdlHLuc/+r6jrkHXQ3KZWKWzzBtQ/DBSLVA2r9jHL5YM8 FXXHIjBjmYJkqzY5q/CT44RGlVuVWXJU85eKE415sWTrnv/ALW96keH/hxO//Hh4vc3X Ot19M1bO1jSxmCxbcjP7dOMHz9WNIAVV5FF6aAOxcmifxguqDepttcIe3IMna8M/0z5R 1bH8/DoFVFTBsjS7VvHV/rSnvZuwBV0XDhxQlYzihYt7fOvATwvmTzlYNVnYoJRrfXYd wVukO6HKBok1XluXm8V/BmvxUL+epbxvwSBXpCsOf9S5e9VgEq3YUuB165itDTLr+Lmc hj5Q== X-Gm-Message-State: ANoB5pmOQYhBRi1EIYgeE57vAm0Y5fI2QpoYkDVGt08/xdAU09Gc/3e1 GZt7Em8SavLpjETwqBU0lSOd/vPMsHLgBwsDESc= X-Google-Smtp-Source: AA0mqf5YISBmUS6i1mJI6d4ufn0nAJlDVmZSupmTdNhLSoqkawH21GyA5Zu+9wvVkuWQY4xZRk7zDnCETzf6vK8WCeA= X-Received: by 2002:a05:6512:376c:b0:4ad:70c1:de61 with SMTP id z12-20020a056512376c00b004ad70c1de61mr30074417lft.509.1670495482713; Thu, 08 Dec 2022 02:31:22 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Thu, 8 Dec 2022 11:31:10 +0100 Message-ID: Subject: Re: [PATCH] cfgbuild: Fix DEBUG_INSN handling in find_bb_boundaries [PR106719] To: Jakub Jelinek Cc: Richard Biener , Jeff Law , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-1.1 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 autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, Dec 8, 2022 at 11:12 AM Jakub Jelinek via Gcc-patches wrote: > > Hi! > > The following testcase FAILs on aarch64-linux. We have some atomic > instruction followed by 2 DEBUG_INSNs (if -g only of course) followed > by NOTE_INSN_EPILOGUE_BEG followed by some USE insn. > Now, split3 pass replaces the atomic instruction with a code sequence > which ends with a conditional jump and the split3 pass calls > find_many_sub_basic_blocks. > For -g0, find_bb_boundaries sees the flow_transfer_insn (the new conditional > jump), then NOTE_INSN_EPILOGUE_BEG which can live in between basic blocks > and then the USE insn, so splits block after the NOTE_INSN_EPILOGUE_BEG > and puts the NOTE in between the blocks. > For -g, if sees a DEBUG_INSN after the flow_transfer_insn, so sets > debug_insn to it, then walks over another DEBUG_INSN, NOTE_INSN_EPILOGUE_BEG > until it finally sees the USE insn, and triggers the: > rtx_insn *prev = PREV_INSN (insn); > > /* If the first non-debug inside_basic_block_p insn after a control > flow transfer is not a label, split the block before the debug > insn instead of before the non-debug insn, so that the debug > insns are not lost. */ > if (debug_insn && code != CODE_LABEL && code != BARRIER) > prev = PREV_INSN (debug_insn); > code I've added for PR81325. If there are only DEBUG_INSNs, that is > the right thing to do, but if in between debug_insn and insn there are > notes which can stay in between basic blocks or simnilarly JUMP_TABLE_DATA > or their associated CODE_LABELs, it causes -fcompare-debug differences. > > The following patch fixes it by clearing debug_insn if JUMP_TABLE_DATA > or associated CODE_LABEL is seen (I'm afraid there is no good answer > what to do with DEBUG_INSNs before those; the code then removes them: > /* Clean up the bb field for the insns between the blocks. */ > for (x = NEXT_INSN (flow_transfer_insn); > x != BB_HEAD (fallthru->dest); > x = next) > { > next = NEXT_INSN (x); > /* Debug insns should not be in between basic blocks, > drop them on the floor. */ > if (DEBUG_INSN_P (x)) > delete_insn (x); > else if (!BARRIER_P (x)) > set_block_for_insn (x, NULL); > } > but if there are NOTEs, the patch just reorders the NOTEs and DEBUG_INSNs, > such that the NOTEs come first (so that they stay in between basic blocks > like with -g0) and DEBUG_INSNs after those (so that bb is split before > them, so they will be in the basic block after NOTE_INSN_BASIC_BLOCK). > > Bootstrapped/regtested on x86_64-linux and i686-linux plus tested on > the testcase in a cross to aarch64-linux, ok for trunk? OK. Thanks, Richard. > 2022-12-08 Jakub Jelinek > > PR debug/106719 > * cfgbuild.cc (find_bb_boundaries): If there are NOTEs in between > debug_insn (seen after flow_transfer_insn) and insn, move NOTEs > before all the DEBUG_INSNs and split after NOTEs. If there are > other insns like jump table data, clear debug_insn. > > * gcc.dg/pr106719.c: New test. > > --- gcc/cfgbuild.cc.jj 2022-01-18 11:58:58.944991171 +0100 > +++ gcc/cfgbuild.cc 2022-12-07 21:36:27.493363173 +0100 > @@ -445,6 +445,7 @@ find_bb_boundaries (basic_block bb) > rtx_insn *debug_insn = NULL; > edge fallthru = NULL; > bool skip_purge; > + bool seen_note_after_debug = false; > > if (insn == end) > return; > @@ -492,7 +493,10 @@ find_bb_boundaries (basic_block bb) > if (code == DEBUG_INSN) > { > if (flow_transfer_insn && !debug_insn) > - debug_insn = insn; > + { > + debug_insn = insn; > + seen_note_after_debug = false; > + } > } > /* In case we've previously seen an insn that effects a control > flow transfer, split the block. */ > @@ -506,7 +510,40 @@ find_bb_boundaries (basic_block bb) > insn instead of before the non-debug insn, so that the debug > insns are not lost. */ > if (debug_insn && code != CODE_LABEL && code != BARRIER) > - prev = PREV_INSN (debug_insn); > + { > + prev = PREV_INSN (debug_insn); > + if (seen_note_after_debug) > + { > + /* Though, if there are NOTEs intermixed with DEBUG_INSNs, > + move the NOTEs before the DEBUG_INSNs and split after > + the last NOTE. */ > + rtx_insn *first = NULL, *last = NULL; > + for (x = debug_insn; x != insn; x = NEXT_INSN (x)) > + { > + if (NOTE_P (x)) > + { > + if (first == NULL) > + first = x; > + last = x; > + } > + else > + { > + gcc_assert (DEBUG_INSN_P (x)); > + if (first) > + { > + reorder_insns_nobb (first, last, prev); > + prev = last; > + first = last = NULL; > + } > + } > + } > + if (first) > + { > + reorder_insns_nobb (first, last, prev); > + prev = last; > + } > + } > + } > fallthru = split_block (bb, prev); > if (flow_transfer_insn) > { > @@ -547,6 +584,14 @@ find_bb_boundaries (basic_block bb) > flow_transfer_insn = prev_nonnote_nondebug_insn_bb (insn); > debug_insn = NULL; > } > + else if (debug_insn) > + { > + if (code == NOTE) > + seen_note_after_debug = true; > + else > + /* Jump tables. */ > + debug_insn = NULL; > + } > > if (control_flow_insn_p (insn)) > flow_transfer_insn = insn; > --- gcc/testsuite/gcc.dg/pr106719.c.jj 2022-12-07 21:35:56.523810192 +0100 > +++ gcc/testsuite/gcc.dg/pr106719.c 2022-12-07 21:35:40.964034788 +0100 > @@ -0,0 +1,13 @@ > +/* PR debug/106719 */ > +/* { dg-do compile { target sync_char_short } } */ > +/* { dg-options "-O2 -fcompare-debug" } */ > + > +extern short int esi, easi[2]; > + > +void > +foo (void) > +{ > + short int *psi = &easi[1]; > + __atomic_nand_fetch (psi, esi, 0); > + psi = &easi[1]; > +} > > Jakub >