From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender-0.a4lg.com [IPv6:2401:2500:203:30b:4000:6bfe:4757:0]) by sourceware.org (Postfix) with ESMTPS id 509E43857C6F for ; Mon, 24 Oct 2022 08:58:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 509E43857C6F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=irq.a4lg.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=irq.a4lg.com Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id E2C58300089; Mon, 24 Oct 2022 08:58:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=irq.a4lg.com; s=2017s01; t=1666601936; bh=c0DiOgW99JnAUYY181cJvRrfAe+9ycFfB+BvqPz33s0=; h=Message-ID:Date:Mime-Version:Subject:To:References:From: In-Reply-To:Content-Type:Content-Transfer-Encoding; b=ln/ckpgHvfsJing4II7nXjE9lasxKb8+ZiqjDgzuMaD/RaNNrKG62+dg1r/CCsk/j ZNGZEG6OIHW/Xd/4N6wRM+E8KhC06p36qDv2bEOwqaHp3e/BtygomHBtea2f6FEB4O JkrxMsixi7YDixCRvQTSxBL7BTqlLOJ2ItcxnLhQ= Message-ID: <8b1d6bee-36e6-b859-0950-8e4fa400964a@irq.a4lg.com> Date: Mon, 24 Oct 2022 17:58:53 +0900 Mime-Version: 1.0 Subject: Re: [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror Content-Language: en-US To: Andrew Burgess , gdb-patches@sourceware.org References: <42c09bcd56bb7bf0a84d58ffad71894f284b5401.1666192979.git.aburgess@redhat.com> <22060850-e52e-afd7-5828-84b9cc061e31@irq.a4lg.com> <87h6zx9ni4.fsf@redhat.com> From: Tsukasa OI In-Reply-To: <87h6zx9ni4.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,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 2022/10/22 0:58, Andrew Burgess wrote: > Tsukasa OI 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. >> >> 2. >> >> 3. >> >> 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. >> >> >> 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: >> >> >> 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 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 >