Re: [gem5-dev] include order style checker bug
Thanks for looking into it. I can't really follow up very easily since I'm travelling, but I'm looking forward to a fix :-). I think there are lots of files which don't quite follow the rules since things were looser in the early days, and even when there were rules established we didn't have a checker for them right away. It's good to fix things up when there's an opportunity, or if the style checker doesn't like your change because of the code it touches. Gabe On Sun, May 5, 2019 at 5:43 PM Zigerelli, Andrew D wrote: > Include order workaround: > @@ -363,6 +363,19 @@ > new = list(self.sort_includes(old, norm_fname, language)) > > modified = _modified_regions(old, new) & regions > +if modified: > +gem_path = os.environ['GEM5_PATH'] > +new_filename = os.path.join(gem_path, 'new.txt') > +old_filename = os.path.join(gem_path, 'old.txt') > +# remove old versions > +with open(new_filename, "a") as f: > +f.write(filename + " new\n") > +for l in new: > +f.write(l+'\n') > +with open(old_filename, "a") as f: > +f.write(filename + " old\n") > +for l in old: > +f.write(l+'\n') > > I just make the changes manually by looking at the difference between old > and new. > > There seems to be a mismatch between what verifiers.py expects to change, > and what actually gets changed. I think the bug has to do with ordering > internal to verifiers.py and ordering outside of it. > > Sorry for being vague; I didn't actually track down why the mismatch > happens. > > Andrew > > -Original Message- > From: gem5-dev On Behalf Of Gambord, Ryan > Sent: Friday, May 3, 2019 10:15 PM > To: gem5 Developer List > Subject: Re: [gem5-dev] include order style checker bug > > I can confirm it's been doing this for a while now on my end. Lots of > existing files in gem5 fail the style checker when I modify them, so it > seems to be at least somewhat recent, or people were overriding the style > checker in the past. > > Ryan Gambord > > On Thu, May 2, 2019, 22:58 Gabe Black wrote: > > > Here's another potentially related bug. There was an extra space > > between the final header and the using at the top of a .cc, and the > > style fixer decided to move the include of the corresponding .hh from > > the top where it belonged down into the list of includes in alphabetic > > order. It still complained about the headers after, but when told to > > fix it it made no changes. It seems the fixer doesn't always recognize > > when the .hh corresponding to a .cc needs to be at the top. > > > > Gabe > > > > On Thu, May 2, 2019 at 10:50 PM Gabe Black wrote: > > > > > Hey folks. I just ran into a bug in the style checker/fixer, and > > > since I wanted to make sure I kept track of those so they can be > > > fixed I thought > > I > > > would describe it here for the record. I have a cc file which had a > > single > > > system include (#include ) and the include for its .hh file > > > (#include "base/loader/loader.hh"), but they were in the wrong > > > order, system and then .hh. The style checker correctly complained > > > about the > > order > > > and offered to fix it, but when I said yes it didn't actually change > > > anything or print any messages. FYI in case somebody wants to > > investigate. > > > > > > Gabe > > > > > ___ > > gem5-dev mailing list > > gem5-dev@gem5.org > > https://nam05.safelinks.protection.outlook.com/?url=http%3A%2F%2Fm5sim > > .org%2Fmailman%2Flistinfo%2Fgem5-devdata=02%7C01%7Canz37%40pitt.e > > du%7C482071dff3564cce9b4808d6d0364833%7C9ef9f489e0a04eeb87cc3a526112fd > > 0d%7C1%7C0%7C636925328891312282sdata=wkeXznipig94J2ytrXFAhWJhPQDZ > > lxxSdQ4f6Oa18dI%3Dreserved=0 > ___ > gem5-dev mailing list > gem5-dev@gem5.org > > https://nam05.safelinks.protection.outlook.com/?url=http%3A%2F%2Fm5sim.org%2Fmailman%2Flistinfo%2Fgem5-devdata=02%7C01%7Canz37%40pitt.edu%7C482071dff3564cce9b4808d6d0364833%7C9ef9f489e0a04eeb87cc3a526112fd0d%7C1%7C0%7C636925328891312282sdata=wkeXznipig94J2ytrXFAhWJhPQDZlxxSdQ4f6Oa18dI%3Dreserved=0 > ___ > gem5-dev mailing list > gem5-dev@gem5.org > http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Change in gem5/gem5[master]: sim-se: correct statfs inclusion on !linux host
Brandon Potter has submitted this change and it was merged. (
https://gem5-review.googlesource.com/c/public/gem5/+/18668 )
Change subject: sim-se: correct statfs inclusion on !linux host
..
sim-se: correct statfs inclusion on !linux host
- Added missing header
- Fixed typo on __linux__ macro conditional
- s/ifdef/if defined/g for consistency
Change-Id: I83b69856e5ec8b23b707642c0e14216cf62db31e
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/18668
Reviewed-by: Jason Lowe-Power
Reviewed-by: Brandon Potter
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/sim/syscall_emul.cc
M src/sim/syscall_emul.hh
2 files changed, 9 insertions(+), 6 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, but someone else must approve; Looks
good to me, approved
Brandon Potter: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/sim/syscall_emul.cc b/src/sim/syscall_emul.cc
index 98fbe96..ba84250 100644
--- a/src/sim/syscall_emul.cc
+++ b/src/sim/syscall_emul.cc
@@ -1039,7 +1039,7 @@
SyscallReturn
fallocateFunc(SyscallDesc *desc, int callnum, Process *p, ThreadContext
*tc)
{
-#if __linux__
+#if defined(__linux__)
int index = 0;
int tgt_fd = p->getSyscallArg(tc, index);
int mode = p->getSyscallArg(tc, index);
diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh
index 91db9ae..caa4d2c 100644
--- a/src/sim/syscall_emul.hh
+++ b/src/sim/syscall_emul.hh
@@ -59,10 +59,13 @@
/// This file defines objects used to emulate syscalls from the target
/// application on the host machine.
-#ifdef __linux__
+#if defined(__linux__)
#include
#include
+#else
+#include
+
#endif
#ifdef __CYGWIN32__
@@ -778,12 +781,12 @@
return status;
}
case SIOCGIFFLAGS:
-#ifdef __linux__
+#if defined(__linux__)
case SIOCGIFINDEX:
#endif
case SIOCGIFNETMASK:
case SIOCGIFADDR:
-#ifdef __linux__
+#if defined(__linux__)
case SIOCGIFHWADDR:
#endif
case SIOCGIFMTU: {
@@ -1515,7 +1518,7 @@
statfsFunc(SyscallDesc *desc, int callnum, Process *process,
ThreadContext *tc)
{
-#ifdef __linux__
+#if defined(__linux__)
std::string path;
int index = 0;
@@ -2851,7 +2854,7 @@
SyscallReturn
eventfdFunc(SyscallDesc *desc, int num, Process *p, ThreadContext *tc)
{
-#ifdef __linux__
+#if defined(__linux__)
int index = 0;
unsigned initval = p->getSyscallArg(tc, index);
int in_flags = p->getSyscallArg(tc, index);
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/18668
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: I83b69856e5ec8b23b707642c0e14216cf62db31e
Gerrit-Change-Number: 18668
Gerrit-PatchSet: 3
Gerrit-Owner: Andrea Mondelli
Gerrit-Reviewer: Brandon Potter
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Continuous integration is live!
Hi Jason, The kokoro environment provides many installed software including clang toolchain. If you need a more complete list of available SW, LMK. /usr/bin $ ls -l clang* lrwxrwxrwx 1 root root 25 Feb 12 2015 clang-3.5 -> ../lib/llvm-3.5/bin/clang lrwxrwxrwx 1 root root 27 Feb 12 2015 clang++-3.5 -> ../lib/llvm-3.5/bin/clang++ lrwxrwxrwx 1 root root 25 Jul 23 2018 clang-3.6 -> ../lib/llvm-3.6/bin/clang lrwxrwxrwx 1 root root 27 Jul 23 2018 clang++-3.6 -> ../lib/llvm-3.6/bin/clang++ lrwxrwxrwx 1 root root 25 Jul 18 2017 clang-3.8 -> ../lib/llvm-3.8/bin/clang lrwxrwxrwx 1 root root 27 Jul 18 2017 clang++-3.8 -> ../lib/llvm-3.8/bin/clang++ lrwxrwxrwx 1 root root 25 Jul 26 2017 clang-3.9 -> ../lib/llvm-3.9/bin/clang lrwxrwxrwx 1 root root 27 Jul 26 2017 clang++-3.9 -> ../lib/llvm-3.9/bin/clang++ lrwxrwxrwx 1 root root 44 Feb 12 2015 clang-apply-replacements-3.5 -> ../lib/llvm-3.5/bin/clang-apply-replacements lrwxrwxrwx 1 root root 44 Jul 23 2018 clang-apply-replacements-3.6 -> ../lib/llvm-3.6/bin/clang-apply-replacements lrwxrwxrwx 1 root root 44 Jul 18 2017 clang-apply-replacements-3.8 -> ../lib/llvm-3.8/bin/clang-apply-replacements lrwxrwxrwx 1 root root 44 Jul 26 2017 clang-apply-replacements-3.9 -> ../lib/llvm-3.9/bin/clang-apply-replacements lrwxrwxrwx 1 root root 31 Feb 12 2015 clang-check-3.5 -> ../lib/llvm-3.5/bin/clang-check lrwxrwxrwx 1 root root 31 Jul 23 2018 clang-check-3.6 -> ../lib/llvm-3.6/bin/clang-check lrwxrwxrwx 1 root root 31 Jul 18 2017 clang-check-3.8 -> ../lib/llvm-3.8/bin/clang-check lrwxrwxrwx 1 root root 31 Jul 26 2017 clang-check-3.9 -> ../lib/llvm-3.9/bin/clang-check lrwxrwxrwx 1 root root 28 Jul 18 2017 clang-cl-3.8 -> ../lib/llvm-3.8/bin/clang-cl lrwxrwxrwx 1 root root 28 Jul 26 2017 clang-cl-3.9 -> ../lib/llvm-3.9/bin/clang-cl lrwxrwxrwx 1 root root 39 Jul 26 2017 clang-include-fixer-3.9 -> ../lib/llvm-3.9/bin/clang-include-fixer lrwxrwxrwx 1 root root 31 Feb 12 2015 clang-query-3.5 -> ../lib/llvm-3.5/bin/clang-query lrwxrwxrwx 1 root root 31 Jul 23 2018 clang-query-3.6 -> ../lib/llvm-3.6/bin/clang-query lrwxrwxrwx 1 root root 31 Jul 18 2017 clang-query-3.8 -> ../lib/llvm-3.8/bin/clang-query lrwxrwxrwx 1 root root 31 Jul 26 2017 clang-query-3.9 -> ../lib/llvm-3.9/bin/clang-query lrwxrwxrwx 1 root root 32 Jul 23 2018 clang-rename-3.6 -> ../lib/llvm-3.6/bin/clang-rename lrwxrwxrwx 1 root root 32 Jul 18 2017 clang-rename-3.8 -> ../lib/llvm-3.8/bin/clang-rename lrwxrwxrwx 1 root root 32 Jul 26 2017 clang-rename-3.9 -> ../lib/llvm-3.9/bin/clang-rename lrwxrwxrwx 1 root root 32 Feb 12 2015 clang-tblgen-3.5 -> ../lib/llvm-3.5/bin/clang-tblgen lrwxrwxrwx 1 root root 32 Jul 23 2018 clang-tblgen-3.6 -> ../lib/llvm-3.6/bin/clang-tblgen lrwxrwxrwx 1 root root 32 Jul 26 2017 clang-tblgen-3.9 -> ../lib/llvm-3.9/bin/clang-tblgen lrwxrwxrwx 1 root root 30 Feb 12 2015 clang-tidy-3.5 -> ../lib/llvm-3.5/bin/clang-tidy lrwxrwxrwx 1 root root 30 Jul 23 2018 clang-tidy-3.6 -> ../lib/llvm-3.6/bin/clang-tidy FYI, kokoro can use custom VM images or docker (via GCR) if there is need to change environment. I can grant limited users (only from gem5-admins ACL) access to a test-debug VM to check the kokoro GCP Ubuntu platform/env. Please email me your ssh public key. Thanks, Rahul. On Fri, May 3, 2019 at 9:53 AM Jason Lowe-Power wrote: > Thanks for the info, Rahul! > > Is there any way to specify a different compiler (e.g., clang) when > building on kokoro? > > Thanks, > Jason > > On Fri, May 3, 2019 at 9:12 AM Rahul Thakur wrote: > >> Hi Giacomo, Jason, >> >> Sorry for late reply. >> Re: tool chain - scons and python are available in the test environment >> AFAIK. >> Re: test throughput - How long does kokoro take to finish current >> presubmit test? >> >> If you have much longer tests to run at periodic frequency, say once in a >> few days, on HEAD, kokoro also has a periodic continuous build mode. Such >> test will send out email report, serving as a pulse check for ToT. >> Presubmit jobs will not be influenced. I can look in to periodic builds if >> it fits for gem5 long regression use case. >> >> RT >> >> On Fri, May 3, 2019 at 6:17 AM Giacomo Travaglini < >> giacomo.travagl...@arm.com> wrote: >> >>> Hi Jason, >>> >>> I have seen patches being under review for a long time, and IMHO adding >>> an extra 10-30 mins is not the real bottleneck. >>> I'd rather wait a little bit more but being sure I am not breaking >>> anything... >>> >>> about ruby protocols, let me say that we (in arm) are relatively happy >>> with the current setup: >>> >>> 1) Quick regressions are run on a commit base (your presubmit.sh). This >>> means it won't take a lot of time/computation >>> 2) Long regressions (ruby protocols are tested here) are run every night >>> on a batch of MERGED commits. >>> Which means at a certain point we just checkout origin/HEAD and we run >>> long regressions. >>> >>> This is the setup I would actually recommend: a sanity suite (quick) >>> being run on a
