Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move Common.py to module #1607

Merged
merged 48 commits into from
Feb 13, 2025
Merged

Move Common.py to module #1607

merged 48 commits into from
Feb 13, 2025

Conversation

bstefanuk
Copy link
Contributor

Moves Common.py into a module, breaking up various elements into self-contained files.

@bstefanuk
feat: no global working path with asserts
d7f3133
@bstefanuk
fix: bad import statement
a1375c7
@bstefanuk
fix: different build_tmp dir in cmake than tensile
07cd30c
@bstefanuk
fix: incorrect pathing in Tensile benchmarking
b5b0f49
@bstefanuk
fix: string wrap paths for assert checks
17aa51a
@bstefanuk
Merge branch 'develop' into remove-global-working-path
33aea17
@bstefanuk
fix: ensure all paths are built
66c157d
@bstefanuk
fix: missing global parameter warning
c2e09c6
@bstefanuk
Merge branch 'develop' into remove-global-working-path
5faedfc
@bstefanuk
fix: ensure library logic path is built
d3a11bc
@bstefanuk
fix: client writer paths
9517470
@bstefanuk
fix: use child path for client libraries
4f73598
@bstefanuk
remove PrintTiming from global params
df28683
@bstefanuk
remove another assert
ea37c06
@bstefanuk
restore PrintTiming
8d72b2b
@bstefanuk
Merge branch 'develop' into remove-global-working-path
af18c2e
@bstefanuk
fix: reviewer comments
eab5145
@bstefanuk
refactor: use env var for @timing decorator
5599a46
@bstefanuk
refactor: remove library print debug
54072cc
@bstefanuk
refactor: remove exit after kernel gen option
4265e9c
@bstefanuk
style: remove commented code
08348e1
@bstefanuk
refactor: move common to a module
8c7b0a3
@bstefanuk
feat: remove 'CustomKernelDirectory' global param
e745693
@bstefanuk
Merge branch 'develop' into remove-global-working-path
a6080cf
@bstefanuk
fix: amd clang version global param
20b514b
@bstefanuk
Merge branch 'develop' into remove-global-working-path
6ef1f5c
@bstefanuk
Merge branch 'develop' into remove-global-working-path
1da3cbf
@bstefanuk
Merge branch 'remove-global-working-path' into move-common-to-module
8390470
@bstefanuk bstefanuk self-assigned this Jan 31, 2025
ellosel
ellosel previously approved these changes Feb 9, 2025
View reviewed changes
@KKyang
Copy link
Contributor

KKyang commented Feb 10, 2025

There are a lot of unrelated files in the PR. Can we remove them?

@bstefanuk
Remove unused and static global parameters (#5)
2b4402e 
* fix: purge library-print-debug
* feat: make build paths static
* feat: remove 'SortProblems' global param
* feat: remove 'ExpandRanges' global param
* feat: remove 'WavefrontWidth' global param
* feat: remove 'ValidateLibrary' global param
* feat: remove 'EnableHalf' and 'ClientArgs' global params
* feat: prefer profile decorator over 'Profiler' global param
* chore: remove 'LibraryPrintDebug' from build_client.yaml
* feat: remove 'MaxFileName' from global params
@bstefanuk
Update preferred search paths (#4)
dc7cf35
@bstefanuk
Copy link
Contributor Author

bstefanuk commented Feb 11, 2025 •
edited
Loading

@KKyang You bet, I'll remove the files from the clients/ and library/ directories. These are here because I'm running the pre-commit hook in .githooks/ and they're automatically indexed when I fix merge conflicts. Could I request that those on your team consider using the pre-commit so that this side-effect can be avoided.

jichangjichang
jichangjichang previously approved these changes Feb 11, 2025
View reviewed changes
Copy link
Collaborator

@jichangjichang jichangjichang left a comment

Choose a reason for hiding this comment

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

please get CI passed before merging.

@KKyang
Copy link
Contributor

KKyang commented Feb 11, 2025

@KKyang You bet, I'll remove the files from the clients/ and library/ directories. These are here because I'm running the pre-commit hook in .githooks/ and they're automatically indexed when I fix merge conflicts. Could I request that those on your team consider using the pre-commit so that this side-effect can be avoided.

Files not modified should not have any changes. If you trigger the hook using git commit, it won't affect unmodified files.

@bstefanuk
Copy link
Contributor Author

bstefanuk commented Feb 11, 2025 •
edited
Loading

This isn't true, because the files are touched when resolving a merge conflict, they're still updated by the precommit.

ellosel
ellosel previously approved these changes Feb 11, 2025
View reviewed changes
@bstefanuk bstefanuk dismissed stale reviews from ellosel and jichangjichang via e4939f7 February 12, 2025 17:57
@KKyang KKyang merged commit f45e5b9 into ROCm:develop Feb 13, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KKyang KKyang KKyang approved these changes

@ellosel ellosel ellosel approved these changes

@jichangjichang jichangjichang jichangjichang left review comments

@vin-huang vin-huang Awaiting requested review from vin-huang vin-huang is a code owner

@imcarsonliao imcarsonliao Awaiting requested review from imcarsonliao imcarsonliao is a code owner

@hcman2 hcman2 Awaiting requested review from hcman2 hcman2 is a code owner

@Serge45 Serge45 Awaiting requested review from Serge45 Serge45 is a code owner

@Jinp800125 Jinp800125 Awaiting requested review from Jinp800125 Jinp800125 is a code owner

@TonyYHsieh TonyYHsieh Awaiting requested review from TonyYHsieh TonyYHsieh is a code owner

@solaslin solaslin Awaiting requested review from solaslin solaslin is a code owner

Assignees

@bstefanuk bstefanuk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bstefanuk @KKyang @jichangjichang @ellosel