Message ID | 20240502185924.2060196-4-vineetg@rivosinc.com |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A2449384AB5B for <patchwork@sourceware.org>; Thu, 2 May 2024 19:05:38 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pl1-x631.google.com (mail-pl1-x631.google.com [IPv6:2607:f8b0:4864:20::631]) by sourceware.org (Postfix) with ESMTPS id 05E0E3849AD6 for <gcc-patches@gcc.gnu.org>; Thu, 2 May 2024 18:59:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 05E0E3849AD6 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 05E0E3849AD6 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::631 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714676377; cv=none; b=op4czjx2DJmBlycRXd5lr7mM6S8ibi1eon/Z24WBqPFTASvxjQQOC69DsAD6IaBkASfV6nXlp3e0miJ+SnUGOz85d1P/uF1xeT5RtHU43H76ysHSj2KC5A65hvfNUj4rbDdB9OALiqS20MscJw2e9pUYGmsiYtu5IZmncwumCHM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714676377; c=relaxed/simple; bh=efhMo9j7i53yumr3+b7JU9cgwvZnFsovzWQCIVgesFM=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=YqNKOSP046q6Felkgw4eMdNw4l0LpBDEt4n5cVe2L2Kzcxsz6ZSCbMt1cu1SGBCS7oS48jSzq2p4aSprSv+TcEv0FEklvW5N9fEwHjlW+Y1g8XH4k+xQnnFP+YMw/2d+XeCd9H6gvkjJgBc1lzJYqAZVjfL6QTVRmLtBGUMNC6A= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x631.google.com with SMTP id d9443c01a7336-1e3ca546d40so71942915ad.3 for <gcc-patches@gcc.gnu.org>; Thu, 02 May 2024 11:59:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1714676374; x=1715281174; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=xUvIyTnVBQN+buGnVUoIPcfOfPMbFOs7JqRhOBFKesk=; b=tK31lO8mx/q0DJNH6tkY26fleCbd5I0lFn7XC2XFq/K8dMKrZHUdR8cV19/B07Ir4g Ise7QEJV66oFmmi+X+YGqhu2CUVO2ckD0OaYWI+3qMf9SUZdWdE931/pWb93xfDHjnFA O2p3UHJAEDI2fzqtj16EGQ/MfWf277a82yNXjg0JwUmA2KsmRr0PUJCZtfj6gZG4/F34 v09uvtDBbZHgKCNWVEWGudXDp4yG+FhyZK7I8EuyBGFbw1AMEms2ctJqniM1oDscz5rx 0dNQXrN9VpnvmckNNRH/xH0h7B0ThPMkToc/25bd8RfMLX5iAlzzF/m/T50uZ2BWnIQ6 uFjw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714676374; x=1715281174; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=xUvIyTnVBQN+buGnVUoIPcfOfPMbFOs7JqRhOBFKesk=; b=rKc0P4eZesVUyBo8VV/i4/mSuW+ok381GdKjD1B4/YesmBss0/nsicH2ghOFSxINpX 3M4npGfTpz6i8zwPDsQyqsDGDcLa7IELtllqFYA9wRF5EKAYtSewkKv+mBXLfbMMqHje v/8GJRWF+g1Fnb9Q4MYpcpqX5jGkvxHGR5bjNXsL6odlBqBHRWopePOqGwmQJ7PNQnof Jwt9eyTXW0mLKaFvp9SRFxXNNlNP7C5tuaumBoFMmGAZnIFzCVrtwpOg3IAfEe3aKygE tS8Kzz3Yti2k/Z3WVj/2E1gPNiuDp8z1HQHgavT2/v+VpK6ColgJrLLcUJQneJZE2io4 qoew== X-Gm-Message-State: AOJu0YzMUHQdiQ3rHhNUfWn11IUZRejnteac3Gled3FGw5HNkwf6IE41 eF3fEmzCY8Mp1jnV5UsyE4zB73I7XECBCRZOvmCh69OOV1aWwjHI63/gX9jE5i91/aj51LHcJ61 KJk8= X-Google-Smtp-Source: AGHT+IF9yeRGD38JI51J/2+FQUVS6fJJ9xXzxXJkZYtuL0qeOXt28JXto2ctkKmoQKFxsrt8+fimPg== X-Received: by 2002:a17:902:ecd2:b0:1eb:7832:8d93 with SMTP id a18-20020a170902ecd200b001eb78328d93mr616653plh.23.1714676373908; Thu, 02 May 2024 11:59:33 -0700 (PDT) Received: from vineet-framework.ba.rivosinc.com ([50.145.13.30]) by smtp.gmail.com with ESMTPSA id l3-20020a170902d04300b001ec83bdfe78sm1667287pll.63.2024.05.02.11.59.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 May 2024 11:59:33 -0700 (PDT) From: Vineet Gupta <vineetg@rivosinc.com> To: gcc-patches@gcc.gnu.org Cc: Jeff Law <jeffreyalaw@gmail.com>, kito.cheng@gmail.com, Palmer Dabbelt <palmer@rivosinc.com>, Robin Dapp <rdapp.gcc@gmail.com>, gnu-toolchain@rivosinc.com, Vineet Gupta <vineetg@rivosinc.com>, Segher Boessenkool <segher@kernel.crashing.org> Subject: [PATCH 3/3] combine: initialize a local var Date: Thu, 2 May 2024 11:59:24 -0700 Message-Id: <20240502185924.2060196-4-vineetg@rivosinc.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240502185924.2060196-1-vineetg@rivosinc.com> References: <20240502185924.2060196-1-vineetg@rivosinc.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org |
Series |
Miscll fixlets
|
|
Checks
Context | Check | Description |
---|---|---|
linaro-tcwg-bot/tcwg_gcc_build--master-arm | success | Testing passed |
linaro-tcwg-bot/tcwg_gcc_check--master-arm | success | Testing passed |
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 | success | Testing passed |
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 | success | Testing passed |
Commit Message
Vineet Gupta
May 2, 2024, 6:59 p.m. UTC
This is no logic change (but technically still a functional change).
Ran into this when stepping thru combine code.
@newpat has some random garbage for a bit until it is actually set.
With the fix it remains 0 until actually set.
gcc/ChangeLog:
* combine.cc (try_combine): Initialize newpat.
CC: Segher Boessenkool <segher@kernel.crashing.org>
Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
gcc/combine.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 5/2/24 12:59 PM, Vineet Gupta wrote: > This is no logic change (but technically still a functional change). > > Ran into this when stepping thru combine code. > @newpat has some random garbage for a bit until it is actually set. > With the fix it remains 0 until actually set. > > gcc/ChangeLog: > * combine.cc (try_combine): Initialize newpat. Isn't the same true even after this change if you turn on the optimizer? And isn't the same true for many other objects that are initialized lazily? Ultimately I'll defer to Segher on this. jeff
On 5/2/24 13:38, Jeff Law wrote: > > On 5/2/24 12:59 PM, Vineet Gupta wrote: >> This is no logic change (but technically still a functional change). >> >> Ran into this when stepping thru combine code. >> @newpat has some random garbage for a bit until it is actually set. >> With the fix it remains 0 until actually set. >> >> gcc/ChangeLog: >> * combine.cc (try_combine): Initialize newpat. > Isn't the same true even after this change if you turn on the optimizer? You are right. And the reason I was seeing a difference at all is all my local toolchain builds are -g3 -O0 :-) > And isn't the same true for many other objects that are initialized > lazily? > > Ultimately I'll defer to Segher on this. Of course. Thx for taking a look. -Vineet
On Thu, May 02, 2024 at 11:59:24AM -0700, Vineet Gupta wrote: > This is no logic change (but technically still a functional change). Where are 1/3 and 2/3? Or are those unrelated? Please don't make series like that. > Ran into this when stepping thru combine code. > @newpat has some random garbage for a bit until it is actually set. > With the fix it remains 0 until actually set. The same is true for all uninitialised variables. Setting everything to zero explicitly is a) quite a bit slower, and b) just as wrong! For example, here, newpat should never be zero. Never. It does not make any sense. Is there any place where newpat is used uninitialised? Segher
On Thu, May 02, 2024 at 02:38:12PM -0600, Jeff Law wrote: > > > On 5/2/24 12:59 PM, Vineet Gupta wrote: > >This is no logic change (but technically still a functional change). > > > >Ran into this when stepping thru combine code. > >@newpat has some random garbage for a bit until it is actually set. > >With the fix it remains 0 until actually set. > > > >gcc/ChangeLog: > > * combine.cc (try_combine): Initialize newpat. > Isn't the same true even after this change if you turn on the optimizer? > And isn't the same true for many other objects that are initialized > lazily? For all, even. Without this change the compiler can (in theory, anyway) diagnose the uninitialised use. After, there *is* no uninitialised use anymore. Please don't do this. It is not an improvement, it is several steps back, to satisfy a misguides sense of "security". Segher
On 5/3/24 01:26, Segher Boessenkool wrote: > On Thu, May 02, 2024 at 11:59:24AM -0700, Vineet Gupta wrote: >> This is no logic change (but technically still a functional change). > Where are 1/3 and 2/3? Or are those unrelated? Yes they were unrelated (minor doc fixes) hence didn't want to spam you, but you are right just because they were piled up in my spring cleaning branch doesn't mean I send them out that way. > Please don't make series like that. Noted for future. > >> Ran into this when stepping thru combine code. >> @newpat has some random garbage for a bit until it is actually set. >> With the fix it remains 0 until actually set. > The same is true for all uninitialised variables. Setting everything > to zero explicitly is a) quite a bit slower, and b) just as wrong! > For example, here, newpat should never be zero. Never. It does not > make any sense. > > Is there any place where newpat is used uninitialised? As I mentioned this patch was the outcome of my "debugging" experience and agree I should not conflate between what the compiler does and what is needed to debug the compiler itself. I've dropped this one. Thx, -Vineet
diff --git a/gcc/combine.cc b/gcc/combine.cc index 92b8d98e6c15..0b5fe00c8c5b 100644 --- a/gcc/combine.cc +++ b/gcc/combine.cc @@ -2522,7 +2522,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0, bool *new_direct_jump_p, rtx_insn *last_combined_insn) { /* New patterns for I3 and I2, respectively. */ - rtx newpat, newi2pat = 0; + rtx newpat = 0, newi2pat = 0; rtvec newpat_vec_with_clobbers = 0; bool substed_i2 = false, substed_i1 = false, substed_i0 = false; /* Indicates need to preserve SET in I0, I1 or I2 in I3 if it is not