Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/make make: convert GNodeFlags from enum into bit-fields



details:   https://anonhg.NetBSD.org/src/rev/b90eb0e6f816
branches:  trunk
changeset: 1026558:b90eb0e6f816
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sun Nov 28 19:51:06 2021 +0000

description:
make: convert GNodeFlags from enum into bit-fields

Now that Enum_ToString is implemented for each type separately, it's
easy to convert them to bit-fields.  This gets rid of the magic numbers
12 for CYCLE and 13 for DONECYCLE that left a suspicious gap in the
numbers.  This gap was not needed since the code didn't make use of the
relative ordering of the enum constants.

The effects of this conversion are fewer capital letters in the code,
smaller scope for the GNode flags, and clearer code especially when
setting a flag back to false.

One strange thing is that GCC 10.3.0 doesn't optimize GNodeFlags_IsNone
to an single bitmasking instruction, at least on x86_64.  Instead it
generates a testb instruction for each of the flags, even loading bit 8
separately from the others.  Clang 12.0.1 knows this optimization
though and generates the obvious sequence of movzwl, testl, jz.

No functional change.

diffstat:

 usr.bin/make/arch.c   |   8 ++--
 usr.bin/make/compat.c |  22 +++++++-------
 usr.bin/make/dir.c    |   6 ++--
 usr.bin/make/job.c    |   6 ++--
 usr.bin/make/make.c   |  74 +++++++++++++++++++++++++-------------------------
 usr.bin/make/make.h   |  25 ++++++++--------
 usr.bin/make/targ.c   |  24 +++++++++++++---
 7 files changed, 89 insertions(+), 76 deletions(-)

diffs (truncated from 517 to 300 lines):

diff -r 4d2f26dc4192 -r b90eb0e6f816 usr.bin/make/arch.c
--- a/usr.bin/make/arch.c       Sun Nov 28 18:58:58 2021 +0000
+++ b/usr.bin/make/arch.c       Sun Nov 28 19:51:06 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: arch.c,v 1.203 2021/08/25 22:14:38 rillig Exp $        */
+/*     $NetBSD: arch.c,v 1.204 2021/11/28 19:51:06 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -126,7 +126,7 @@
 #include "config.h"
 
 /*     "@(#)arch.c     8.2 (Berkeley) 1/2/94"  */
-MAKE_RCSID("$NetBSD: arch.c,v 1.203 2021/08/25 22:14:38 rillig Exp $");
+MAKE_RCSID("$NetBSD: arch.c,v 1.204 2021/11/28 19:51:06 rillig Exp $");
 
 typedef struct List ArchList;
 typedef struct ListNode ArchListNode;
@@ -944,12 +944,12 @@
                        const char *nameEnd = strchr(nameStart, ')');
                        size_t nameLen = (size_t)(nameEnd - nameStart);
 
-                       if ((pgn->flags & REMAKE) &&
+                       if (pgn->flags.remake &&
                            strncmp(nameStart, gn->name, nameLen) == 0) {
                                Arch_UpdateMTime(pgn);
                                gn->mtime = pgn->mtime;
                        }
-               } else if (pgn->flags & REMAKE) {
+               } else if (pgn->flags.remake) {
                        /*
                         * Something which isn't a library depends on the
                         * existence of this target, so it needs to exist.
diff -r 4d2f26dc4192 -r b90eb0e6f816 usr.bin/make/compat.c
--- a/usr.bin/make/compat.c     Sun Nov 28 18:58:58 2021 +0000
+++ b/usr.bin/make/compat.c     Sun Nov 28 19:51:06 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: compat.c,v 1.227 2021/04/27 15:19:25 christos Exp $    */
+/*     $NetBSD: compat.c,v 1.228 2021/11/28 19:51:06 rillig Exp $      */
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -96,7 +96,7 @@
 #include "pathnames.h"
 
 /*     "@(#)compat.c   8.2 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: compat.c,v 1.227 2021/04/27 15:19:25 christos Exp $");
+MAKE_RCSID("$NetBSD: compat.c,v 1.228 2021/11/28 19:51:06 rillig Exp $");
 
 static GNode *curTarg = NULL;
 static pid_t compatChild;
@@ -494,7 +494,7 @@
         * again. This is our signal to not attempt to do anything but abort
         * our parent as well.
         */
-       gn->flags |= REMAKE;
+       gn->flags.remake = true;
        gn->made = BEINGMADE;
 
        if (!(gn->type & OP_MADE))
@@ -502,9 +502,9 @@
 
        MakeNodes(&gn->children, gn);
 
-       if (!(gn->flags & REMAKE)) {
+       if (!gn->flags.remake) {
                gn->made = ABORTED;
-               pgn->flags &= ~(unsigned)REMAKE;
+               pgn->flags.remake = false;
                return false;
        }
 
@@ -582,13 +582,13 @@
                 */
                gn->made = MADE;
                if (Make_Recheck(gn) == 0)
-                       pgn->flags |= FORCE;
+                       pgn->flags.force = true;
                if (!(gn->type & OP_EXEC)) {
-                       pgn->flags |= CHILDMADE;
+                       pgn->flags.childMade = true;
                        GNode_UpdateYoungestChild(pgn, gn);
                }
        } else if (opts.keepgoing) {
-               pgn->flags &= ~(unsigned)REMAKE;
+               pgn->flags.remake = false;
        } else {
                PrintOnError(gn, "\nStop.");
                exit(1);
@@ -609,11 +609,11 @@
        case BEINGMADE:
                Error("Graph cycles through %s", gn->name);
                gn->made = ERROR;
-               pgn->flags &= ~(unsigned)REMAKE;
+               pgn->flags.remake = false;
                break;
        case MADE:
                if (!(gn->type & OP_EXEC)) {
-                       pgn->flags |= CHILDMADE;
+                       pgn->flags.childMade = true;
                        GNode_UpdateYoungestChild(pgn, gn);
                }
                break;
@@ -660,7 +660,7 @@
                 * Already had an error when making this.
                 * Tell the parent to abort.
                 */
-               pgn->flags &= ~(unsigned)REMAKE;
+               pgn->flags.remake = false;
        } else {
                MakeOther(gn, pgn);
        }
diff -r 4d2f26dc4192 -r b90eb0e6f816 usr.bin/make/dir.c
--- a/usr.bin/make/dir.c        Sun Nov 28 18:58:58 2021 +0000
+++ b/usr.bin/make/dir.c        Sun Nov 28 19:51:06 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: dir.c,v 1.273 2021/09/21 21:39:32 rillig Exp $ */
+/*     $NetBSD: dir.c,v 1.274 2021/11/28 19:51:06 rillig Exp $ */
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -138,7 +138,7 @@
 #include "job.h"
 
 /*     "@(#)dir.c      8.2 (Berkeley) 1/2/94"  */
-MAKE_RCSID("$NetBSD: dir.c,v 1.273 2021/09/21 21:39:32 rillig Exp $");
+MAKE_RCSID("$NetBSD: dir.c,v 1.274 2021/11/28 19:51:06 rillig Exp $");
 
 /*
  * A search path is a list of CachedDir structures. A CachedDir has in it the
@@ -1447,7 +1447,7 @@
 
                fullName = Dir_FindFile(gn->name, Suff_FindPath(gn));
 
-               if (fullName == NULL && gn->flags & FROM_DEPEND &&
+               if (fullName == NULL && gn->flags.fromDepend &&
                    !Lst_IsEmpty(&gn->implicitParents))
                        fullName = ResolveMovedDepends(gn);
 
diff -r 4d2f26dc4192 -r b90eb0e6f816 usr.bin/make/job.c
--- a/usr.bin/make/job.c        Sun Nov 28 18:58:58 2021 +0000
+++ b/usr.bin/make/job.c        Sun Nov 28 19:51:06 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: job.c,v 1.439 2021/11/28 17:26:07 rillig Exp $ */
+/*     $NetBSD: job.c,v 1.440 2021/11/28 19:51:06 rillig Exp $ */
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -142,7 +142,7 @@
 #include "trace.h"
 
 /*     "@(#)job.c      8.2 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: job.c,v 1.439 2021/11/28 17:26:07 rillig Exp $");
+MAKE_RCSID("$NetBSD: job.c,v 1.440 2021/11/28 19:51:06 rillig Exp $");
 
 /*
  * A shell defines how the commands are run.  All commands for a target are
@@ -1367,7 +1367,7 @@
         * this node's parents so they never get examined.
         */
 
-       if (gn->flags & FROM_DEPEND) {
+       if (gn->flags.fromDepend) {
                if (!Job_RunTarget(".STALE", gn->fname))
                        fprintf(stdout,
                            "%s: %s, %d: ignoring stale %s for %s\n",
diff -r 4d2f26dc4192 -r b90eb0e6f816 usr.bin/make/make.c
--- a/usr.bin/make/make.c       Sun Nov 28 18:58:58 2021 +0000
+++ b/usr.bin/make/make.c       Sun Nov 28 19:51:06 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: make.c,v 1.245 2021/11/28 18:58:58 rillig Exp $        */
+/*     $NetBSD: make.c,v 1.246 2021/11/28 19:51:06 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -104,7 +104,7 @@
 #include "job.h"
 
 /*     "@(#)make.c     8.1 (Berkeley) 6/6/93"  */
-MAKE_RCSID("$NetBSD: make.c,v 1.245 2021/11/28 18:58:58 rillig Exp $");
+MAKE_RCSID("$NetBSD: make.c,v 1.246 2021/11/28 19:51:06 rillig Exp $");
 
 /* Sequence # to detect recursion. */
 static unsigned int checked_seqno = 1;
@@ -196,16 +196,16 @@
        Buffer buf;
 
        Buf_InitSize(&buf, 32);
-#define ADD(flag) Buf_AddFlag(&buf, (flags & (flag)) != 0, #flag)
-       ADD(REMAKE);
-       ADD(CHILDMADE);
-       ADD(FORCE);
-       ADD(DONE_WAIT);
-       ADD(DONE_ORDER);
-       ADD(FROM_DEPEND);
-       ADD(DONE_ALLSRC);
-       ADD(CYCLE);
-       ADD(DONECYCLE);
+#define ADD(flag, name) Buf_AddFlag(&buf, flags.flag, name)
+       ADD(remake, "REMAKE");
+       ADD(childMade, "CHILDMADE");
+       ADD(force, "FORCE");
+       ADD(doneWait, "DONE_WAIT");
+       ADD(doneOrder, "DONE_ORDER");
+       ADD(fromDepend, "FROM_DEPEND");
+       ADD(doneAllsrc, "DONE_ALLSRC");
+       ADD(cycle, "CYCLE");
+       ADD(doneCycle, "DONECYCLE");
 #undef ADD
        return buf.len == 0 ? "none" : (*freeIt = Buf_DoneData(&buf));
 }
@@ -344,8 +344,8 @@
                 */
                DEBUG0(MAKE, ".JOIN node...");
                DEBUG1(MAKE, "source %smade...",
-                   gn->flags & CHILDMADE ? "" : "not ");
-               oodate = (gn->flags & CHILDMADE) != 0;
+                   gn->flags.childMade ? "" : "not ");
+               oodate = gn->flags.childMade;
        } else if (gn->type & (OP_FORCE | OP_EXEC | OP_PHONY)) {
                /*
                 * A node which is the object of the force (!) operator or
@@ -373,10 +373,10 @@
                 * child after it was considered made.
                 */
                if (DEBUG(MAKE)) {
-                       if (gn->flags & FORCE)
+                       if (gn->flags.force)
                                debug_printf("non existing child...");
                }
-               oodate = (gn->flags & FORCE) != 0;
+               oodate = gn->flags.force;
        }
 
 #ifdef USE_META
@@ -626,7 +626,7 @@
 
        for (ln = cgn->implicitParents.first; ln != NULL; ln = ln->next) {
                GNode *pgn = ln->datum;
-               if (pgn->flags & REMAKE) {
+               if (pgn->flags.remake) {
                        Var_Set(pgn, IMPSRC, cname);
                        if (cpref != NULL)
                                Var_Set(pgn, PREFIX, cpref);
@@ -643,7 +643,7 @@
        for (ln = gn->order_pred.first; ln != NULL; ln = ln->next) {
                GNode *ogn = ln->datum;
 
-               if (GNode_IsDone(ogn) || !(ogn->flags & REMAKE))
+               if (GNode_IsDone(ogn) || !ogn->flags.remake)
                        continue;
 
                DEBUG2(MAKE,
@@ -742,13 +742,13 @@
                        debug_printf(", unmade %d ", pgn->unmade - 1);
                }
 
-               if (!(pgn->flags & REMAKE)) {
+               if (!pgn->flags.remake) {
                        /* This parent isn't needed */
                        DEBUG0(MAKE, "- not needed\n");
                        continue;
                }
                if (mtime == 0 && !(cgn->type & OP_WAIT))
-                       pgn->flags |= FORCE;
+                       pgn->flags.force = true;
 
                /*
                 * If the parent has the .MADE attribute, its timestamp got
@@ -765,7 +765,7 @@
 
                if (!(cgn->type & (OP_EXEC | OP_USE | OP_USEBEFORE))) {
                        if (cgn->made == MADE)
-                               pgn->flags |= CHILDMADE;
+                               pgn->flags.childMade = true;
                        GNode_UpdateYoungestChild(pgn, cgn);
                }
 
@@ -798,7 +798,7 @@
                 * nodes.
                 */
                if (pgn->unmade != 0 && !(centurion->type & OP_WAIT)
-                   && !(centurion->flags & DONE_ORDER)) {
+                   && !centurion->flags.doneOrder) {
                        DEBUG0(MAKE, "- unmade children\n");
                        continue;
                }
@@ -931,7 +931,7 @@
 {
        GNodeListNode *ln;
 
-       if (gn->flags & DONE_ALLSRC)
+       if (gn->flags.doneAllsrc)
                return;
 
        UnmarkChildren(gn);
@@ -945,7 +945,7 @@
 
        if (gn->type & OP_JOIN)
                Var_Set(gn, TARGET, GNode_VarAllsrc(gn));



Home | Main Index | Thread Index | Old Index