Skip to content

Enable cross-directory ccache#1539

Merged
lockshaw merged 7 commits into
flexflow:masterfrom
lockshaw:ccache-build-test-1
Jan 15, 2025
Merged

Enable cross-directory ccache#1539
lockshaw merged 7 commits into
flexflow:masterfrom
lockshaw:ccache-build-test-1

Conversation

@lockshaw

@lockshaw lockshaw commented Nov 10, 2024

Copy link
Copy Markdown
Collaborator

Description of changes:

The current ccache setup is very limited in its ability to cache between directories due to the fact that the compiler-generated debug info and any __FILE__ macros cause builds to differ between directories, which is quite annoying if you are using ff-dev and would like to quickly ffpr down a PR and build it as you end up building from scratch. This PR adds the necessary compiler flags to make debug info and __FILE__ macros use relative paths, and then adds a wrapper around gdb that ensures that the debugging experience isn't affected (the only limitation is that you have to be running the debugger inside the nix environment, which you essentially have to do anyway).

Related Issues:

Linked Issues:

  • Issue #

Issues closed by this PR:

  • Closes #

This change is Reviewable

@lockshaw
lockshaw marked this pull request as ready for review November 10, 2024 03:23
@lockshaw
lockshaw changed the base branch from repo-refactor to master December 16, 2024 08:36

@Marsella8 Marsella8 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

From my very limited understanding of nix, everything seems good.

Reviewed 7 of 8 files at r1, all commit messages.
Reviewable status: 7 of 8 files reviewed, 2 unresolved discussions (waiting on @lockshaw and @reyna-abhyankar)


.flake/pkgs/ffdb/default.nix line 32 at r1 (raw file):

        python3
      ]}
    cp $out/bin/gdb $out/bin/ffdb

so this effectively auto-executes ffdb.py before every gdb call right?


.flake/pkgs/ffdb/ffdb.py line 9 at r1 (raw file):

gdb.execute('set history save on')

# python

The commented out code should either be deleted or updated? (e.g. MachineViewPrinter still refers to the old interface).

@lockshaw lockshaw left a comment

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.

Reviewable status: 6 of 8 files reviewed, 2 unresolved discussions (waiting on @Marsella8 and @reyna-abhyankar)


.flake/pkgs/ffdb/default.nix line 32 at r1 (raw file):

Previously, Marsella8 wrote…

so this effectively auto-executes ffdb.py before every gdb call right?

Correct


.flake/pkgs/ffdb/ffdb.py line 9 at r1 (raw file):

Previously, Marsella8 wrote…

The commented out code should either be deleted or updated? (e.g. MachineViewPrinter still refers to the old interface).

Done.

@lockshaw
lockshaw enabled auto-merge (squash) January 15, 2025 02:43
@lockshaw
lockshaw merged commit 5e7f207 into flexflow:master Jan 15, 2025
@codecov

codecov Bot commented Jan 15, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (master@030bfd6). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #1539   +/-   ##
=========================================
  Coverage          ?   78.50%           
=========================================
  Files             ?      867           
  Lines             ?    28904           
  Branches          ?      778           
=========================================
  Hits              ?    22690           
  Misses            ?     6214           
  Partials          ?        0           
Flag Coverage Δ
unittests 78.50% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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.

2 participants