Skip to content

[Build] Rework Makefile rules to ease addition of LOC package#6

Merged
gapisback merged 1 commit intomainfrom
gapisback/l3-LOC-initial
Mar 13, 2024
Merged

[Build] Rework Makefile rules to ease addition of LOC package#6
gapisback merged 1 commit intomainfrom
gapisback/l3-LOC-initial

Conversation

@gapisback
Copy link
Copy Markdown
Collaborator

@gapisback gapisback commented Mar 10, 2024

This commit refactors the Makefile rules to ease the addition of rules to integrate Line-of-Code (LOC) package in a follow-on commit. Several redundant TEST_BIN etc. symbols are purged.

Existing capabilities are unchanged, and we now additionally support:

$ make clean && CC=g++ CXX=g++ LD=g++ make all
$ make run-tests

... to build all .c, .cpp and .cc sample-programs and run them in one-pass.

Added test.sh wrapper script for devs to quickly re-run all the tests that are run in CI, for easily stabilizing changes w/o having to go thru CI.


Updated make help usage messages (as a quick reference):

agurajada-Linux-Vm:[1559] $ make help

Usage:

To build all sample programs and run all unit-tests:
 make clean && CC=g++ CXX=g++ LD=g++ make all
 make run-tests

Alternatively, you may use multiple invocations in this sequence to build all sources:

To build C-sample programs and run unit-tests:
 make clean && CC=gcc LD=g++ make all-c-tests
 make run-c-tests

To build C++-sample .cpp programs and run unit-tests:
 make clean-l3 && CC=g++ CXX=g++ LD=g++ make all-cpp-tests
 make run-cpp-tests

To build C++-sample .cc programs and run unit-tests:
 make clean-l3 && CC=g++ CXX=g++ LD=g++ make all-cc-tests
 make run-cc-tests

Environment variables:
 BUILD_MODE={release,debug}
 BUILD_VERBOSE={0,1}

NOTE: To the reviewer:

This is really a non-functional, code-cleanup, refactoring change.

I have folded-in the support for LOC package on top of this code in a test PR #8 , which shows that this refactoring greatly simplifies adding in Makefile rules to pick-up LOC's generated files and other Make-rules.

Additionally, in this change-set, you will see LOC making an appearance in l3.h and l3.c files:

I have edited the files to "prepare" the structures and L3 APIs to start working with 4-byte loc_t ID value.

Currently, all this is #ifdef'ed out under L3_LOC_ENABLED define.

In a future PR, the builds will be invoked with this -DL3_LOC_ENABLED which will - magically - activate this code, and L3 logging will become LOC-enabled.

Once this goes in, I am ready to submit a PR for integrating LOC with L3 package.

@gapisback gapisback force-pushed the gapisback/l3-LOC-initial branch 3 times, most recently from 2d8e373 to 09f1efa Compare March 13, 2024 06:09
@gapisback gapisback changed the title Integrate L3 with LOC tooling machinery. [Build] Rework Makefile rules to ease addition of LOC package Mar 13, 2024

- name: Build-CC-Samples
run: BUILD_MODE=${{ matrix.build_type }} make clean-l3 && BUILD_MODE=${{ matrix.build_type }} CC=g++ CXX=g++ LD=g++ make all-cc-tests

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new tests added to CI. Just adding blank lines for readability.

Comment thread Makefile
@echo 'Usage:'
@echo ' '
@echo 'To build all sample programs and run all unit-tests:'
@echo ' make clean && CC=g++ CXX=g++ LD=g++ make all'
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New functionality being onlined here.

Comment thread Makefile
# test-use-cases/single-file-Cpp-program/test-main.cpp
#
# Convert this list of *main.c to generate test-code program binary names,
# using the src-dir's name as the program name.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these TEST* symbols on L137 were found unnecessary, so I've simplified them.

Comment thread Makefile
SINGLE_FILE_CC_PROGRAM_OBJS := $(SINGLE_FILE_CC_PROGRAM_OBJS:%.c=$(OBJDIR)/%.o)

$(BINDIR)/$(TESTSDIR)/single-file-CC-program: $(SINGLE_FILE_CC_PROGRAM_OBJS)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole deleted block is now unnecessary as the dependencies of make targets on sample program binaries is defined differently, above.

Comment thread include/l3.h

#include <stdint.h>

#ifdef L3_LOC_ENABLED
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here onwards, in the change-set, you will see LOC making an appearance.

I have edited the files to "prepare" the structures and L3 APIs to start working with 4-byte loc_t ID value.

Currently, all this is #ifdef'ed out under L3_LOC_ENABLED define.

In a future PR, the builds will be invoked with this -DL3_LOC_ENABLED which will - magically - activate this code, and L3 logging will become LOC-enabled.

Comment thread include/l3.h
*/
#ifdef L3_LOC_ENABLED
#define l3_log_simple(msg, arg1, arg2) \
l3__log_simple(__LOC__, (msg), (arg1), (arg2))
Copy link
Copy Markdown
Collaborator Author

@gapisback gapisback Mar 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an immediately upcoming PR #8 , if we build the sample programs with L3_LOC_ENABLED=1, then we will store the LOC-ID token in the L3 logging entry struct. Otherwise, will just store 0.

Comment thread src/l3.c
{
pid_t tid;
#ifdef L3_LOC_ENABLED
loc_t loc;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that in a future PR, when this field is activated, we are -only- re-using the 4-byte hole that is conveniently left behind for us in current non-L3_LOC_ENABLED build:

(gdb) ptype/o struct l3_entry
/* offset      |    size */  type = struct l3_entry {
/*      0      |       4 */    pid_t tid;
/* XXX  4-byte hole      */                                <<< loc_t loc; field is using these pad bytes.
/*      8      |       8 */    const char *msg;
/*     16      |       8 */    uint64_t arg1;
/*     24      |       8 */    uint64_t arg2;

                               /* total size (bytes):   32 */
                             }

@gapisback gapisback requested a review from gregthelaw March 13, 2024 06:26
@gapisback gapisback marked this pull request as ready for review March 13, 2024 06:46
@gapisback gapisback force-pushed the gapisback/l3-LOC-initial branch from 09f1efa to 986fda5 Compare March 13, 2024 13:21
This commit refactors the Makefile rules to ease the addition
of rules to integrate Line-of-Code (LOC) package in a follow-on
commit. Several redundant TEST_*BIN* etc. symbols are purged.

Existing capabilities are unchanged, and we now additionally
support:
$ make clean && CC=g++ CXX=g++ LD=g++ make all
$ make run-tests

... to build all .c, .cpp and .cc sample-programs and run them
in one-pass.

clean: rm /tmp/l3*.dat files, also.
Update `make help` usage messages to be more self-explanatory.
@gapisback gapisback force-pushed the gapisback/l3-LOC-initial branch from 986fda5 to efa685b Compare March 13, 2024 13:31
@gapisback gapisback merged commit 8283e76 into main Mar 13, 2024
@gapisback gapisback deleted the gapisback/l3-LOC-initial branch March 13, 2024 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant