From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 307C3385843B for ; Thu, 2 Sep 2021 15:08:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 307C3385843B Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-56-1nxwHyAsNle08v0WFkmNBw-1; Thu, 02 Sep 2021 11:08:28 -0400 X-MC-Unique: 1nxwHyAsNle08v0WFkmNBw-1 Received: by mail-qk1-f197.google.com with SMTP id c27-20020a05620a165b00b003d3817c7c23so1858587qko.16 for ; Thu, 02 Sep 2021 08:08:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=0kjXPy0I7wpwFtx9wC7K+YXhHofs7uIyhDoOQVfdBw0=; b=DFxVXWBmgVj8KMA9kHoBnyMlWnTcNV3ebBZyaItezd4GiPH4fJ1xgDbeehOqt1A6BX A0Jyt3FkJWV6eoAUwzs5F2bbb8CLcnTeIiGrpv+JT46wtlFJ8myS/nkmoggyc3LdMN1O 93Yvc9HaAZU+DnaUVIPadfdZMcdIwa1Luxu5tEpShISlijdzk7MZ0B2z6szBMWX07YpK Fgp6dydVoMtpXup5V/nzA+mI4lCewHbaDy1k/u0XKMgN+dEbkgBaKrIqqu400LGhfpG9 JgWQ1eOalR+GG2+jK7vWBvIOnmk92Tk5StkrmsKJZaYY5ZEfvUcIlh8TAXYnxyTGNGEB /imw== X-Gm-Message-State: AOAM5322pdSLZjj8VdAgo2Vspi1W/G3TINMTrCtyxT5fafRZHViPs+j2 wwLibdfa7Y0NhPZC9nFttINLVputM1AHHVF3LyBNIxoPAjAC1hOr9+LfZ66YFgVoXkEa+4su3gu ioI3wwo8= X-Received: by 2002:a05:6214:1142:: with SMTP id b2mr3543130qvt.0.1630595308120; Thu, 02 Sep 2021 08:08:28 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwaLXZKN+o0JJqM2Jn2gRQ3W+Tyc4TxeInpLr8R+ENl7FaaGa8CJw2N1r7jRmxxOyaGYV7E9A== X-Received: by 2002:a05:6214:1142:: with SMTP id b2mr3543100qvt.0.1630595307840; Thu, 02 Sep 2021 08:08:27 -0700 (PDT) Received: from t14s.localdomain (c-73-69-212-193.hsd1.ma.comcast.net. [73.69.212.193]) by smtp.gmail.com with ESMTPSA id b13sm1159248qtb.13.2021.09.02.08.08.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Sep 2021 08:08:27 -0700 (PDT) Message-ID: <175ff404ebdbb82ea24385209be83c3793da44cd.camel@redhat.com> Subject: Re: [PATCH] jit : Generate debug info for variables From: David Malcolm To: Petter Tomner , "gcc-patches@gcc.gnu.org" , "jit@gcc.gnu.org" Date: Thu, 02 Sep 2021 11:08:26 -0400 In-Reply-To: <005bad3cb21745d591cea63567d08d08@kth.se> References: <005bad3cb21745d591cea63567d08d08@kth.se> User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: jit@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Jit mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Sep 2021 15:08:31 -0000 On Tue, 2021-08-31 at 00:13 +0000, Petter Tomner via Gcc-patches wrote: > Hi, > > This is a patch to generate debug info for local variables as well as > globals. > With this, "ptype foo", "info variables", "info locals" etc works > when debugging in GDB. > > Finalizing of global variable declares are moved to after locations > are handled and done > as Fortran, C, Go etc do it. Also, primitive types have their > TYPE_NAME set for debug info > on types to work. > > Below are the patch, and I attached a testcase. Since it requires GDB > to run it might > not be suitable? Make check-jit runs fine on Debian x64. Thanks for the patches. Overall, looks good, but I have some review nits... Reviewing patch 1 in this email... > From d77e77104024c7ae9ce31b419dad1f0a5801fda7 Mon Sep 17 00:00:00 > 2001 > From: Petter Tomner > Date: Mon, 30 Aug 2021 01:44:07 +0200 > Subject: [PATCH 1/2] libgccjit: Generate debug info for variables > > Finalize declares via available helpers after location is set. Set > TYPE_NAME of primitives and friends to "int" etc. Debug info is now > set properly for variables. > > 2021-08-31      Petter Tomner    > > gcc/jit >         jit-playback.c >         jit-playback.h > gcc/testsuite/jit.dg/ >         test-error-array-bounds.c: Array is not unsigned Can you write non-empty ChangeLog entries please. [...snip...] > --- a/gcc/jit/jit-playback.c > +++ b/gcc/jit/jit-playback.c [...snip...] > @@ -2984,15 +2975,22 @@ replay () >      { >        int i; >        function *func; > - > +      tree global; >        /* No GC can happen yet; process the cached source locations.  > */ >        handle_locations (); >   > +      /* Finalize globals. See how FORTRAN 95 does it in > gfc_be_parse_file() > +         for a simple reference. */ > +      FOR_EACH_VEC_ELT (m_globals, i, global) > +    rest_of_decl_compilation (global, true, true); > + > +      wrapup_global_declarations (m_globals.address(), > m_globals.length()); > + >        /* We've now created tree nodes for the stmts in the various > blocks > -        in each function, but we haven't built each function's > single stmt > -        list yet.  Do so now.  */ > +              in each function, but we haven't built each function's > single stmt > +              list yet.  Do so now.  */ >        FOR_EACH_VEC_ELT (m_functions, i, func) > -       func->build_stmt_list (); > +         func->build_stmt_list (); Looks like some whitespace churn above; did your text editor accidentally convert tabs to spaces? I prefer to avoid changes that touch lines without changing things, as it messes up e.g. "git blame". In case you haven't discovered it yet, "git add -p" is very helpful for just staging individual hunks within a changed file. [...snip...] ...plus some comments about the testcase which I'll post in reply to the other patch. Dave