Message ID | 42c09bcd56bb7bf0a84d58ffad71894f284b5401.1666192979.git.aburgess@redhat.com |
---|---|
State | Committed |
Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.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 9E7373856DF1 for <patchwork@sourceware.org>; Wed, 19 Oct 2022 15:30:11 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9E7373856DF1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666193411; bh=xvQiIxZ5kLlechieVNtLr2rl5b3azTxbUDztKJalIbc=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=BKCRa28z5udy96iJcnjaoIi3sEAUbvU72+9VGfESN/vxc0vMWiFklmISjYGB3DdUg hF2fsyh8rjGHa4cQOHApi7NjzIOv8y0KzkFIO/pZmM0VcHt6mtr8VXWT90tIax9jRH msvwO4EYyREBFI3iodwaLAfLiy918PMjnCkFxR3w= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id DBE1D385086D for <gdb-patches@sourceware.org>; Wed, 19 Oct 2022 15:25:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DBE1D385086D Received: from mail-qv1-f72.google.com (mail-qv1-f72.google.com [209.85.219.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-108-JdncMQQGOoSyH_0GFBIz4g-1; Wed, 19 Oct 2022 11:25:14 -0400 X-MC-Unique: JdncMQQGOoSyH_0GFBIz4g-1 Received: by mail-qv1-f72.google.com with SMTP id lu3-20020a0562145a0300b004b1d6f4130eso10768543qvb.1 for <gdb-patches@sourceware.org>; Wed, 19 Oct 2022 08:25:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=xvQiIxZ5kLlechieVNtLr2rl5b3azTxbUDztKJalIbc=; b=ZDdN6kVyG0Xe2X7a9XQNG3oK3C3RUBON70K2Xh6zXVUEFlDnO+DLCP2Kl/XeIP21tO 6pB+5a7SueLC/p9rh0rJCz+7kafPulHVa6UPhqSPlwQwUdaHIFd8YYlWO6UQc8Sbo+tY 0smQnuEdwMD1e7DK069aFH1Qzo4o6VUw+U+JINLi8xEYD2q4SmtWCYhkJwtLzO5m8Qs+ V6f+Fw7Z8hA3MApr7ckOxpVvEY/HLXSX/N8qz7lu9PV+rFFda9D1MrCX1Nbg6+pqZ+h/ 2p/5sGPM8cAzh/fyzCZIUZ4qLSflAX9K0rOvKs0BBZ/r/yA31Luj5keUhL31xNAku1sa 272g== X-Gm-Message-State: ACrzQf2jbKm37HNhigcdgPkMmN0Hx8NGJkKIWRwkldz3YTsbXFzCzGkb vNhDxqeilxMVJdIi2JYw/oX3mMfxshjg0rP0hHKVCUfZBjh3uLzF33iyNz3m1PYU+BFd2wCPVSN IhqrLaWSHzFWhdSqOrX9mJ379ew7BAHd3zCGBz43dfMSwGCNUJNz6BM9Me/gO4GpetNpzE7nYGw == X-Received: by 2002:a05:622a:286:b0:39c:d621:610b with SMTP id z6-20020a05622a028600b0039cd621610bmr6771127qtw.608.1666193114429; Wed, 19 Oct 2022 08:25:14 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7bjo/l9Mos0cQx2oauMLHTz5Axi5hUlhrmrE/TVnJT1RdpleUSD/WV5OxJaWJoSufcmNR1zA== X-Received: by 2002:a05:622a:286:b0:39c:d621:610b with SMTP id z6-20020a05622a028600b0039cd621610bmr6771109qtw.608.1666193114154; Wed, 19 Oct 2022 08:25:14 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id cm24-20020a05622a251800b0039492d503cdsm4267422qtb.51.2022.10.19.08.25.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 19 Oct 2022 08:25:13 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror Date: Wed, 19 Oct 2022 16:24:49 +0100 Message-Id: <42c09bcd56bb7bf0a84d58ffad71894f284b5401.1666192979.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: <cover.1666192979.git.aburgess@redhat.com> References: <cover.1666192979.git.aburgess@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, 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: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> Reply-To: Andrew Burgess <aburgess@redhat.com> Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
Building the sim/ tree with clang
|
|
Commit Message
Andrew Burgess
Oct. 19, 2022, 3:24 p.m. UTC
When building the cris, m32c, and sh simulators with Clang I am seeing build warnings from a few objects. These three simulators currently build with -Werror, and so these warnings cause the build to fail. When built with gcc I don't see any warnings from these targets, so the -Werror is fine. As the warnings are not new, in this commit, I propose that we disable the use of -Werror for these three simulators. With this done it is now possible to build the complete simulator tree using clang. --- sim/cris/Makefile.in | 3 +++ sim/m32c/Makefile.in | 3 +++ sim/sh/Makefile.in | 3 +++ 3 files changed, 9 insertions(+)
Comments
Hi Andrew, It's surprising for me that you are working on that! Based on your branch, I applied one patch (from my upcoming patchset) each time I met an error (until I see no errors) and here's some results (including simple patches to CGEN-generated files): Required additional patches (from my patchset) to Andrew tree: 1. With PATCH 10/10 - Clang 15.0.3 : 16 (+2 for cpu/cris) 2. With PATCH 10/10 - Clang 15.0.0 : 19 (+2 for cpu/cris) 3. W/O PATCH 10/10 - Clang 15.0.3 : 20 (+2 for cpu/cris) 4. W/O PATCH 10/10 - Clang 15.0.0 : 23 (+2 for cpu/cris) c.f. 1. <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-1> 2. <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-2> 3. <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-3> 4. <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-4> Environment: - Ubuntu 22.04.1 LTS - LLVM + Clang 15.0.0 (built from git source; primary) - LLVM + Clang 15.0.3 (built from git source; secondary) And... my tree (alone with my changes) failed on Clang 15.0.0 but succeeded on Clang 15.0.1. That is because -Wimplicit-function-declaration was default-error on 15.0.0 but downgraded to default-warning on 15.0.1. All error messages generated by Clang 15.0.0 makes sense so I'll continue using Clang 15.0.0 as a reference. c.f. <https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213> Thanks to your PATCH 09/10, I could build my working tree with Clang except M32R simulator with Clang 15.0.0. Not only that, I could figure out how to make M32R simulator (sort of) work. For CRIS and SuperH, I can provide cleaner solution without disabling -Werror (at least on my environment). My Current Tree (to be submitted as a RFC PATCH) is available at: <https://github.com/a4lg/binutils-gdb/tree/clang-nowarn-1> My initial plan was to split it to smaller patchsets and submit each ones periodically but... it seems submitting all at once seems syncing / reviewing my and your work easier. I'll submit a RFC patchset consisting of 40 or 41 patches in total. Note that: 1. There are three cpu/* changes (must be reviewed on Binutils side) 2. There are some optional changes (suppresses non-error warnings) 3. PATCH 01 is a duplicate of another patchset (1 patch in total) 4. PATCH 16 is entirely authored by Andrew (PATCH 09/10) and 5. There are other patches that are pretty much the same as Andrew's. They are coincidence because I and Andrew are trying to fix the same issue. Thanks, Tsukasa On 2022/10/20 0:24, Andrew Burgess via Gdb-patches wrote: > When building the cris, m32c, and sh simulators with Clang I am seeing > build warnings from a few objects. These three simulators currently > build with -Werror, and so these warnings cause the build to fail. > > When built with gcc I don't see any warnings from these targets, so > the -Werror is fine. > > As the warnings are not new, in this commit, I propose that we disable > the use of -Werror for these three simulators. With this done it is > now possible to build the complete simulator tree using clang. > --- > sim/cris/Makefile.in | 3 +++ > sim/m32c/Makefile.in | 3 +++ > sim/sh/Makefile.in | 3 +++ > 3 files changed, 9 insertions(+) > > diff --git a/sim/cris/Makefile.in b/sim/cris/Makefile.in > index d58aeee9363..53e485dca02 100644 > --- a/sim/cris/Makefile.in > +++ b/sim/cris/Makefile.in > @@ -41,6 +41,9 @@ SIM_EXTRA_DEPS = \ > > SIM_EXTRA_CLEAN = cris-clean > > +# Some modules don't build cleanly yet. > +mloopv10f.o mloopv32f.o: SIM_WERROR_CFLAGS = > + > ## COMMON_POST_CONFIG_FRAG > > arch = cris > diff --git a/sim/m32c/Makefile.in b/sim/m32c/Makefile.in > index 2436eb940f4..dd9b3aaf175 100644 > --- a/sim/m32c/Makefile.in > +++ b/sim/m32c/Makefile.in > @@ -40,4 +40,7 @@ SIM_OBJS = \ > trace.o \ > $(ENDLIST) > > +# Some modules don't build cleanly yet. > +mem.o: SIM_WERROR_CFLAGS = > + > ## COMMON_POST_CONFIG_FRAG > diff --git a/sim/sh/Makefile.in b/sim/sh/Makefile.in > index fc794f30687..a496095e767 100644 > --- a/sim/sh/Makefile.in > +++ b/sim/sh/Makefile.in > @@ -24,4 +24,7 @@ SIM_OBJS = \ > SIM_EXTRA_LIBS = -lm > SIM_EXTRA_DEPS = table.c code.c ppi.c > > +# Some modules don't build cleanly yet. > +interp.o: SIM_WERROR_CFLAGS = > + > ## COMMON_POST_CONFIG_FRAG
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
Andrew> As the warnings are not new, in this commit, I propose that we disable
Andrew> the use of -Werror for these three simulators. With this done it is
Andrew> now possible to build the complete simulator tree using clang.
This seems fine to me, but I think it would be good if the comments
mentioned clang.
Tom
Tsukasa OI <research_trasio@irq.a4lg.com> writes: > Hi Andrew, > > It's surprising for me that you are working on that! While I was reviewing your previous clang patches I tried to build the sim/ tree with clang, but even with your patches I was hitting a bunch of warnings/errors. I created a set of changes as a fixup so that I could test your patches. That is, this started as the set of extra fixes I needed on top of your work to get the sim/ tree building with clang (for me). Once your work was merged I decided, having done this work, I might as well split the fixes into separate patches and get them merged. I may have expanded slightly by looking at any warnings emitted by gcc as well as any other non-fatal warnings from clang, and fixing anything that looked easy. > > Based on your branch, I applied one patch (from my upcoming patchset) > each time I met an error (until I see no errors) and here's some results > (including simple patches to CGEN-generated files): > > Required additional patches (from my patchset) to Andrew tree: > 1. With PATCH 10/10 - Clang 15.0.3 : 16 (+2 for cpu/cris) > 2. With PATCH 10/10 - Clang 15.0.0 : 19 (+2 for cpu/cris) > 3. W/O PATCH 10/10 - Clang 15.0.3 : 20 (+2 for cpu/cris) > 4. W/O PATCH 10/10 - Clang 15.0.0 : 23 (+2 for cpu/cris) Yeah, my version of clang is somewhat older than that (9.x !) > c.f. > 1. > <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-1> > 2. > <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-2> > 3. > <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-3> > 4. > <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-4> > > Environment: > - Ubuntu 22.04.1 LTS > - LLVM + Clang 15.0.0 (built from git source; primary) > - LLVM + Clang 15.0.3 (built from git source; secondary) > > And... my tree (alone with my changes) failed on Clang 15.0.0 but > succeeded on Clang 15.0.1. That is because > -Wimplicit-function-declaration was default-error on 15.0.0 but > downgraded to default-warning on 15.0.1. All error messages generated > by Clang 15.0.0 makes sense so I'll continue using Clang 15.0.0 as a > reference. > > c.f. > <https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213> > > Thanks to your PATCH 09/10, I could build my working tree with Clang > except M32R simulator with Clang 15.0.0. Not only that, I could figure > out how to make M32R simulator (sort of) work. For CRIS and SuperH, I > can provide cleaner solution without disabling -Werror (at least on my > environment). Yes please. The disable -Werror isn't a final resting place, its more just a stopping point so we can have something that builds - though I think what you're saying above is that with later versions of clang things still don't build, which is a shame. > > My Current Tree (to be submitted as a RFC PATCH) is available at: > <https://github.com/a4lg/binutils-gdb/tree/clang-nowarn-1> > > My initial plan was to split it to smaller patchsets and submit each > ones periodically but... it seems submitting all at once seems syncing / > reviewing my and your work easier. I'll submit a RFC patchset > consisting of 40 or 41 patches in total. It's not clear to me if any of my patches above conflict significantly with patches you have. If there are any of the above that are a problem, then just let me know and I wont merge them. I don't have any plans to do anything more on this in the immediate future, like I said, with the tools I have to hand right now the sim tree now builds fine. I have added revisiting cris/m32c/sh to my todo list, but I doubt that will get looked at before 2023, and it sounds like you might have a solution before then - which would be nice :) Ideally, I'd like to push any of the above patches that don't actively make your life harder, I'll drop anything that is a problem for you. And then look forward to your incoming fixes. Let me know, Thanks, Andrew > > Note that: > > 1. There are three cpu/* changes (must be reviewed on Binutils side) > 2. There are some optional changes (suppresses non-error warnings) > 3. PATCH 01 is a duplicate of another patchset (1 patch in total) > 4. PATCH 16 is entirely authored by Andrew (PATCH 09/10) and > 5. There are other patches that are pretty much the same as Andrew's. > They are coincidence because I and Andrew are trying > to fix the same issue. > > Thanks, > Tsukasa > > On 2022/10/20 0:24, Andrew Burgess via Gdb-patches wrote: >> When building the cris, m32c, and sh simulators with Clang I am seeing >> build warnings from a few objects. These three simulators currently >> build with -Werror, and so these warnings cause the build to fail. >> >> When built with gcc I don't see any warnings from these targets, so >> the -Werror is fine. >> >> As the warnings are not new, in this commit, I propose that we disable >> the use of -Werror for these three simulators. With this done it is >> now possible to build the complete simulator tree using clang. >> --- >> sim/cris/Makefile.in | 3 +++ >> sim/m32c/Makefile.in | 3 +++ >> sim/sh/Makefile.in | 3 +++ >> 3 files changed, 9 insertions(+) >> >> diff --git a/sim/cris/Makefile.in b/sim/cris/Makefile.in >> index d58aeee9363..53e485dca02 100644 >> --- a/sim/cris/Makefile.in >> +++ b/sim/cris/Makefile.in >> @@ -41,6 +41,9 @@ SIM_EXTRA_DEPS = \ >> >> SIM_EXTRA_CLEAN = cris-clean >> >> +# Some modules don't build cleanly yet. >> +mloopv10f.o mloopv32f.o: SIM_WERROR_CFLAGS = >> + >> ## COMMON_POST_CONFIG_FRAG >> >> arch = cris >> diff --git a/sim/m32c/Makefile.in b/sim/m32c/Makefile.in >> index 2436eb940f4..dd9b3aaf175 100644 >> --- a/sim/m32c/Makefile.in >> +++ b/sim/m32c/Makefile.in >> @@ -40,4 +40,7 @@ SIM_OBJS = \ >> trace.o \ >> $(ENDLIST) >> >> +# Some modules don't build cleanly yet. >> +mem.o: SIM_WERROR_CFLAGS = >> + >> ## COMMON_POST_CONFIG_FRAG >> diff --git a/sim/sh/Makefile.in b/sim/sh/Makefile.in >> index fc794f30687..a496095e767 100644 >> --- a/sim/sh/Makefile.in >> +++ b/sim/sh/Makefile.in >> @@ -24,4 +24,7 @@ SIM_OBJS = \ >> SIM_EXTRA_LIBS = -lm >> SIM_EXTRA_DEPS = table.c code.c ppi.c >> >> +# Some modules don't build cleanly yet. >> +interp.o: SIM_WERROR_CFLAGS = >> + >> ## COMMON_POST_CONFIG_FRAG
On 19 Oct 2022 16:24, Andrew Burgess via Gdb-patches wrote: > When building the cris, m32c, and sh simulators with Clang I am seeing > build warnings from a few objects. These three simulators currently > build with -Werror, and so these warnings cause the build to fail. > > When built with gcc I don't see any warnings from these targets, so > the -Werror is fine. > > As the warnings are not new, in this commit, I propose that we disable > the use of -Werror for these three simulators. With this done it is > now possible to build the complete simulator tree using clang. sory, but i don't understand the logic here. the code builds cleanly with gcc which is why we have -Werror enabled. but when building with clang, you see errors, so you want to disable -Werror for gcc and allow new issues to slip in to the tree ? this change/approach looks wrong to me. if the tree is clean with gcc, leave it to people interested in clang to figure it out w/out making the situation worse for GNU users. this is the GNU sim, not the LLVM sim. -mike
On 2022/10/22 0:58, Andrew Burgess wrote: > Tsukasa OI <research_trasio@irq.a4lg.com> writes: > >> Hi Andrew, >> >> It's surprising for me that you are working on that! > > While I was reviewing your previous clang patches I tried to build the > sim/ tree with clang, but even with your patches I was hitting a bunch > of warnings/errors. > > I created a set of changes as a fixup so that I could test your patches. > That is, this started as the set of extra fixes I needed on top of your > work to get the sim/ tree building with clang (for me). > > Once your work was merged I decided, having done this work, I might as > well split the fixes into separate patches and get them merged. I may > have expanded slightly by looking at any warnings emitted by gcc as well > as any other non-fatal warnings from clang, and fixing anything that > looked easy. > >> >> Based on your branch, I applied one patch (from my upcoming patchset) >> each time I met an error (until I see no errors) and here's some results >> (including simple patches to CGEN-generated files): >> >> Required additional patches (from my patchset) to Andrew tree: >> 1. With PATCH 10/10 - Clang 15.0.3 : 16 (+2 for cpu/cris) >> 2. With PATCH 10/10 - Clang 15.0.0 : 19 (+2 for cpu/cris) >> 3. W/O PATCH 10/10 - Clang 15.0.3 : 20 (+2 for cpu/cris) >> 4. W/O PATCH 10/10 - Clang 15.0.0 : 23 (+2 for cpu/cris) > > Yeah, my version of clang is somewhat older than that (9.x !) > >> c.f. >> 1. >> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-1> >> 2. >> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-2> >> 3. >> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-3> >> 4. >> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-4> >> >> Environment: >> - Ubuntu 22.04.1 LTS >> - LLVM + Clang 15.0.0 (built from git source; primary) >> - LLVM + Clang 15.0.3 (built from git source; secondary) >> >> And... my tree (alone with my changes) failed on Clang 15.0.0 but >> succeeded on Clang 15.0.1. That is because >> -Wimplicit-function-declaration was default-error on 15.0.0 but >> downgraded to default-warning on 15.0.1. All error messages generated >> by Clang 15.0.0 makes sense so I'll continue using Clang 15.0.0 as a >> reference. >> >> c.f. >> <https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213> >> >> Thanks to your PATCH 09/10, I could build my working tree with Clang >> except M32R simulator with Clang 15.0.0. Not only that, I could figure >> out how to make M32R simulator (sort of) work. For CRIS and SuperH, I >> can provide cleaner solution without disabling -Werror (at least on my >> environment). > > Yes please. The disable -Werror isn't a final resting place, its more > just a stopping point so we can have something that builds - though I > think what you're saying above is that with later versions of clang > things still don't build, which is a shame. > >> >> My Current Tree (to be submitted as a RFC PATCH) is available at: >> <https://github.com/a4lg/binutils-gdb/tree/clang-nowarn-1> >> >> My initial plan was to split it to smaller patchsets and submit each >> ones periodically but... it seems submitting all at once seems syncing / >> reviewing my and your work easier. I'll submit a RFC patchset >> consisting of 40 or 41 patches in total. > > It's not clear to me if any of my patches above conflict significantly > with patches you have. > > If there are any of the above that are a problem, then just let me know > and I wont merge them. > > I don't have any plans to do anything more on this in the immediate > future, like I said, with the tools I have to hand right now the sim > tree now builds fine. > > I have added revisiting cris/m32c/sh to my todo list, but I doubt that > will get looked at before 2023, and it sounds like you might have a > solution before then - which would be nice :) > > Ideally, I'd like to push any of the above patches that don't actively > make your life harder, I'll drop anything that is a problem for you. > And then look forward to your incoming fixes. > > Let me know, > > Thanks, > Andrew c.f.: my related patchset <https://sourceware.org/pipermail/gdb-patches/2022-October/192853.html> Well... I just submitted my draft RFC PATCH (misnamed [PATCH] though) to sync and discuss with your current/future work. I knew it will take some time (**did** take some time) to investigate Clang issues and I don't want I and you to avoid redoing each other's work. I almost support your patchset (as I describe below). TL;DR: I support merging your ten patches except PATCH 04,07,10/10. Although I personally don't like to write a patch like PATCH 10/10, it might be a good short-term solution for now (I don't object like PATCH 04,07/10). Each review to your patchset is as follows: [+] : Positive [-] : Negative PATCH 01/10: [+] Corresponds to my PATCH 36/40. I'll withdraw my one. PATCH 02/10: [+] Corresponds to my PATCH 30/40. I'll withdraw my one. PATCH 03/10: [+] Corresponds to my PATCH 28/40. Although my PATCH 28/40 is a bit simpler, your patch is easy to understand and I feel this is OK to me. PATCH 04/10: [-] Corresponds to my PATCH 31/40. I prefer my solution (to initialize help with {0}). PATCH 05/10: [+] Corresponds (but very different) to my PATCH 27/40. I chose to keep the semantics as in the original source code but it seems it keeps the bug in this place. Andrew's fixes that bug (I think) and I prefer Andrew's one. PATCH 06/10: [+] Corresponds to my PATCH 03/40. I'll withdraw my one. PATCH 07/10: [-] Corresponds to my PATCH 34/40. Although this function is currently unused, I prefer to keep this function (with ATTRIBUTE_UNUSED) for now. PATCH 08/10: [+] Corresponds to my PATCH 15/40. I chose to add dummy addition (+ 0x0) and you chose just remove the self-assignments. Both looks equally good and it's okay to withdraw my one. PATCH 09/10: [+] Copied to my working branch (as PATCH 16/40). Needless to say, this PATCH 16/40 is yours. PATCH 10/10: [neutral] Personally I don't like this kind of resolution but there's no strong reason to object to this. I received quite a lot of review for bigger batch 1 and will submit v2 after resolving some of this (and removing changes corresponding to your patches except PATCH 04,07/10). Thanks, Tsukasa > >> >> Note that: >> >> 1. There are three cpu/* changes (must be reviewed on Binutils side) >> 2. There are some optional changes (suppresses non-error warnings) >> 3. PATCH 01 is a duplicate of another patchset (1 patch in total) >> 4. PATCH 16 is entirely authored by Andrew (PATCH 09/10) and >> 5. There are other patches that are pretty much the same as Andrew's. >> They are coincidence because I and Andrew are trying >> to fix the same issue. >> >> Thanks, >> Tsukasa >> >> On 2022/10/20 0:24, Andrew Burgess via Gdb-patches wrote: >>> When building the cris, m32c, and sh simulators with Clang I am seeing >>> build warnings from a few objects. These three simulators currently >>> build with -Werror, and so these warnings cause the build to fail. >>> >>> When built with gcc I don't see any warnings from these targets, so >>> the -Werror is fine. >>> >>> As the warnings are not new, in this commit, I propose that we disable >>> the use of -Werror for these three simulators. With this done it is >>> now possible to build the complete simulator tree using clang. >>> --- >>> sim/cris/Makefile.in | 3 +++ >>> sim/m32c/Makefile.in | 3 +++ >>> sim/sh/Makefile.in | 3 +++ >>> 3 files changed, 9 insertions(+) >>> >>> diff --git a/sim/cris/Makefile.in b/sim/cris/Makefile.in >>> index d58aeee9363..53e485dca02 100644 >>> --- a/sim/cris/Makefile.in >>> +++ b/sim/cris/Makefile.in >>> @@ -41,6 +41,9 @@ SIM_EXTRA_DEPS = \ >>> >>> SIM_EXTRA_CLEAN = cris-clean >>> >>> +# Some modules don't build cleanly yet. >>> +mloopv10f.o mloopv32f.o: SIM_WERROR_CFLAGS = >>> + >>> ## COMMON_POST_CONFIG_FRAG >>> >>> arch = cris >>> diff --git a/sim/m32c/Makefile.in b/sim/m32c/Makefile.in >>> index 2436eb940f4..dd9b3aaf175 100644 >>> --- a/sim/m32c/Makefile.in >>> +++ b/sim/m32c/Makefile.in >>> @@ -40,4 +40,7 @@ SIM_OBJS = \ >>> trace.o \ >>> $(ENDLIST) >>> >>> +# Some modules don't build cleanly yet. >>> +mem.o: SIM_WERROR_CFLAGS = >>> + >>> ## COMMON_POST_CONFIG_FRAG >>> diff --git a/sim/sh/Makefile.in b/sim/sh/Makefile.in >>> index fc794f30687..a496095e767 100644 >>> --- a/sim/sh/Makefile.in >>> +++ b/sim/sh/Makefile.in >>> @@ -24,4 +24,7 @@ SIM_OBJS = \ >>> SIM_EXTRA_LIBS = -lm >>> SIM_EXTRA_DEPS = table.c code.c ppi.c >>> >>> +# Some modules don't build cleanly yet. >>> +interp.o: SIM_WERROR_CFLAGS = >>> + >>> ## COMMON_POST_CONFIG_FRAG >
Tsukasa OI <research_trasio@irq.a4lg.com> writes: > On 2022/10/22 0:58, Andrew Burgess wrote: >> Tsukasa OI <research_trasio@irq.a4lg.com> writes: >> >>> Hi Andrew, >>> >>> It's surprising for me that you are working on that! >> >> While I was reviewing your previous clang patches I tried to build the >> sim/ tree with clang, but even with your patches I was hitting a bunch >> of warnings/errors. >> >> I created a set of changes as a fixup so that I could test your patches. >> That is, this started as the set of extra fixes I needed on top of your >> work to get the sim/ tree building with clang (for me). >> >> Once your work was merged I decided, having done this work, I might as >> well split the fixes into separate patches and get them merged. I may >> have expanded slightly by looking at any warnings emitted by gcc as well >> as any other non-fatal warnings from clang, and fixing anything that >> looked easy. >> >>> >>> Based on your branch, I applied one patch (from my upcoming patchset) >>> each time I met an error (until I see no errors) and here's some results >>> (including simple patches to CGEN-generated files): >>> >>> Required additional patches (from my patchset) to Andrew tree: >>> 1. With PATCH 10/10 - Clang 15.0.3 : 16 (+2 for cpu/cris) >>> 2. With PATCH 10/10 - Clang 15.0.0 : 19 (+2 for cpu/cris) >>> 3. W/O PATCH 10/10 - Clang 15.0.3 : 20 (+2 for cpu/cris) >>> 4. W/O PATCH 10/10 - Clang 15.0.0 : 23 (+2 for cpu/cris) >> >> Yeah, my version of clang is somewhat older than that (9.x !) >> >>> c.f. >>> 1. >>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-1> >>> 2. >>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-2> >>> 3. >>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-3> >>> 4. >>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-4> >>> >>> Environment: >>> - Ubuntu 22.04.1 LTS >>> - LLVM + Clang 15.0.0 (built from git source; primary) >>> - LLVM + Clang 15.0.3 (built from git source; secondary) >>> >>> And... my tree (alone with my changes) failed on Clang 15.0.0 but >>> succeeded on Clang 15.0.1. That is because >>> -Wimplicit-function-declaration was default-error on 15.0.0 but >>> downgraded to default-warning on 15.0.1. All error messages generated >>> by Clang 15.0.0 makes sense so I'll continue using Clang 15.0.0 as a >>> reference. >>> >>> c.f. >>> <https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213> >>> >>> Thanks to your PATCH 09/10, I could build my working tree with Clang >>> except M32R simulator with Clang 15.0.0. Not only that, I could figure >>> out how to make M32R simulator (sort of) work. For CRIS and SuperH, I >>> can provide cleaner solution without disabling -Werror (at least on my >>> environment). >> >> Yes please. The disable -Werror isn't a final resting place, its more >> just a stopping point so we can have something that builds - though I >> think what you're saying above is that with later versions of clang >> things still don't build, which is a shame. >> >>> >>> My Current Tree (to be submitted as a RFC PATCH) is available at: >>> <https://github.com/a4lg/binutils-gdb/tree/clang-nowarn-1> >>> >>> My initial plan was to split it to smaller patchsets and submit each >>> ones periodically but... it seems submitting all at once seems syncing / >>> reviewing my and your work easier. I'll submit a RFC patchset >>> consisting of 40 or 41 patches in total. >> >> It's not clear to me if any of my patches above conflict significantly >> with patches you have. >> >> If there are any of the above that are a problem, then just let me know >> and I wont merge them. >> >> I don't have any plans to do anything more on this in the immediate >> future, like I said, with the tools I have to hand right now the sim >> tree now builds fine. >> >> I have added revisiting cris/m32c/sh to my todo list, but I doubt that >> will get looked at before 2023, and it sounds like you might have a >> solution before then - which would be nice :) >> >> Ideally, I'd like to push any of the above patches that don't actively >> make your life harder, I'll drop anything that is a problem for you. >> And then look forward to your incoming fixes. >> >> Let me know, >> >> Thanks, >> Andrew > > c.f.: my related patchset > <https://sourceware.org/pipermail/gdb-patches/2022-October/192853.html> > > > Well... I just submitted my draft RFC PATCH (misnamed [PATCH] though) to > sync and discuss with your current/future work. I knew it will take > some time (**did** take some time) to investigate Clang issues and I > don't want I and you to avoid redoing each other's work. > > I almost support your patchset (as I describe below). > > TL;DR: > I support merging your ten patches except PATCH 04,07,10/10. Although I > personally don't like to write a patch like PATCH 10/10, it might be a > good short-term solution for now (I don't object like PATCH 04,07/10). > > Each review to your patchset is as follows: > [+] : Positive > [-] : Negative > > PATCH 01/10: [+] > Corresponds to my PATCH 36/40. I'll withdraw my one. > PATCH 02/10: [+] > Corresponds to my PATCH 30/40. I'll withdraw my one. > PATCH 03/10: [+] > Corresponds to my PATCH 28/40. Although my PATCH 28/40 is a bit > simpler, your patch is easy to understand and I feel this is OK > to me. > PATCH 04/10: [-] > Corresponds to my PATCH 31/40. I prefer my solution (to > initialize help with {0}). > PATCH 05/10: [+] > Corresponds (but very different) to my PATCH 27/40. > I chose to keep the semantics as in the original source code but > it seems it keeps the bug in this place. Andrew's fixes that bug > (I think) and I prefer Andrew's one. > PATCH 06/10: [+] > Corresponds to my PATCH 03/40. I'll withdraw my one. > PATCH 07/10: [-] > Corresponds to my PATCH 34/40. Although this function is currently > unused, I prefer to keep this function (with ATTRIBUTE_UNUSED) > for now. We don't generally keep dead code around, it's always possible to pull things back from git if needed. Before I push patch #7 I thought I'd ask why you wanted to keep this, but were OK with patch #6. Thanks, Andrew
diff --git a/sim/cris/Makefile.in b/sim/cris/Makefile.in index d58aeee9363..53e485dca02 100644 --- a/sim/cris/Makefile.in +++ b/sim/cris/Makefile.in @@ -41,6 +41,9 @@ SIM_EXTRA_DEPS = \ SIM_EXTRA_CLEAN = cris-clean +# Some modules don't build cleanly yet. +mloopv10f.o mloopv32f.o: SIM_WERROR_CFLAGS = + ## COMMON_POST_CONFIG_FRAG arch = cris diff --git a/sim/m32c/Makefile.in b/sim/m32c/Makefile.in index 2436eb940f4..dd9b3aaf175 100644 --- a/sim/m32c/Makefile.in +++ b/sim/m32c/Makefile.in @@ -40,4 +40,7 @@ SIM_OBJS = \ trace.o \ $(ENDLIST) +# Some modules don't build cleanly yet. +mem.o: SIM_WERROR_CFLAGS = + ## COMMON_POST_CONFIG_FRAG diff --git a/sim/sh/Makefile.in b/sim/sh/Makefile.in index fc794f30687..a496095e767 100644 --- a/sim/sh/Makefile.in +++ b/sim/sh/Makefile.in @@ -24,4 +24,7 @@ SIM_OBJS = \ SIM_EXTRA_LIBS = -lm SIM_EXTRA_DEPS = table.c code.c ppi.c +# Some modules don't build cleanly yet. +interp.o: SIM_WERROR_CFLAGS = + ## COMMON_POST_CONFIG_FRAG