Skip to content

Support Tricore compiler from Tasking - #942

Closed
louiscaron wants to merge 5 commits into
ccache:masterfrom
louiscaron:master
Closed

Support Tricore compiler from Tasking#942
louiscaron wants to merge 5 commits into
ccache:masterfrom
louiscaron:master

Conversation

@louiscaron

@louiscaron louiscaron commented Oct 6, 2021

Copy link
Copy Markdown
Contributor

First draft before full support. This should allow more people to collaborate and use the feature, without forking from my own copy.

This is related to the following issue: #841

@louiscaron louiscaron changed the title Import first draft for Tricore compiler from Tasking Support Tricore compiler from Tasking Oct 7, 2021
@louiscaron

Copy link
Copy Markdown
Contributor Author

I would like to get a first review to make sure that my patch meets the implementation requirements

@louiscaron

Copy link
Copy Markdown
Contributor Author

@jrosdahl : would you mind approving the workflows on my pull request? thanks

@louiscaron

Copy link
Copy Markdown
Contributor Author

@jrosdahl : could I ask you to run the workflows again? thanks

@louiscaron

Copy link
Copy Markdown
Contributor Author

@jrosdahl : could I ask you to run the workflows again? I fixed all the issues reported by the previous run

@louiscaron
louiscaron force-pushed the master branch 3 times, most recently from affbeba to c93f9c3 Compare October 30, 2021 14:26
@louiscaron

Copy link
Copy Markdown
Contributor Author

@jrosdahl could you approve the workflows? Actually, I start having several changes that are independent from each other (tasking support vs windows fixes). I have separated them clearly in commits, but do you want me to have indididual pull requests in order to get them in the mainline? thanks

@louiscaron

Copy link
Copy Markdown
Contributor Author

@jrosdahl : thanks to the compiled unit tests, I discovered a new path format (/Z:/path/to/dir), I fixed the path substitution to match the three formats I have seen (/path/to/dir, /Z:/path/to/dir and Z:/path/to/dir). Could you run the workflows once more? thanks

@jrosdahl

jrosdahl commented Nov 2, 2021

Copy link
Copy Markdown
Member

Thanks for working on this, and especially for taking the time to write tests!

As mentioned in #506 (comment) and #956, I have some reservations regarding adding support for yet another new compiler by following the pre-existing pattern with logic sprinkled here and there. Since this PR brings even more complexity to core functionality, I'm currently not interested in merging this as is. Sorry.

Regarding the tests: They are of course good to have for a developer that has access to a Tasking compiler, but if the test suite isn't run by CI and not by any maintainer (i.e. me) then they won't protect against regression in reality and have a large risk of becoming broken. See #957 for an idea on how this could be improved.

@louiscaron

louiscaron commented Nov 3, 2021

Copy link
Copy Markdown
Contributor Author

@jrosdahl thanks a lot for the long response, if i understand correctly, there is no chance that I get this merged in the mainline as is. But since I can't stand wasted work, I am ready to do anything that can address your concerns, so could I:

  • rework my patch to completely duplicate the portions that are compiler specific so that it does not make the core argument parsing more complex
  • add fake compilers (as python scripts eventually) so the regression tests can pass in CI

Let me know if you have other ideas to get it to be merged, thanks

@jrosdahl

Copy link
Copy Markdown
Member

@jrosdahl thanks a lot for the long response, if i understand correctly, there is no chance that I get this merged in the mainline as is.

I wouldn't go as far as saying that there is no chance, but I currently just don't have time to engage with this low priority (sorry) topic. Just wanted to say that I haven't forgotten about it and that I have nothing against supporting Tricore/Tasking in principle, given a better designed framework.

Let me know if you have other ideas to get it to be merged, thanks

That could be helping with designing or thinking about #956 and #957, or helping out with other maintenance aspects of ccache.

@vsplesk

vsplesk commented Mar 20, 2022

Copy link
Copy Markdown
Contributor

Is there anything we can still do? I merged @louiscaron changes with v4.6 release (mostly mechanistic chanes, not sure about argprocessing.cpp).

See https://github.com/vsplesk/ccache/tree/tricoremerge

@louiscaron

Copy link
Copy Markdown
Contributor Author

Thanks @vsplesk for the merge in the dedicated branch....

@jrosdahl jrosdahl linked an issue Aug 18, 2022 that may be closed by this pull request
Louis Caron and others added 4 commits October 3, 2022 10:25
Support dependency files that are generated with spaces between
the target and the colon sign
Add unit tests to test the tasking tricore ctc/cctc compiler
support.
Comment thread src/argprocessing.cpp
Comment on lines +348 to +353
if ((config.compiler_type() == CompilerType::nvcc
&& (args[i] == "-optf" || args[i] == "--options-file"))
|| ((config.compiler_type() == CompilerType::cctc
|| config.compiler_type() == CompilerType::ctc)
&& (args[i] == "-f"
|| util::starts_with(args[i], "--option-file=")))) {

Check notice

Code scanning / CodeQL

Complex condition

Complex condition: too many logical operations in this expression.
@YYGCui

YYGCui commented Jun 27, 2024

Copy link
Copy Markdown

Hi, is any update for this PR?

@louiscaron I want to use ccache for tasking compiler,pls help me check is it available or not?

@louiscaron

Copy link
Copy Markdown
Contributor Author

@YYGCui : as per a discussion with @jrosdahl , it will not be merged in the mainline for various reasons (adding too much complexity in the compiler options handling, only a commercial compiler so no non regression tests can be added to the main branch etc...)

Nevertheless, you can use my fork which has added support for this compiler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

issue: feature New feature status: won't fix Will not fix/implement/merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for Tasking compiler toolchain

5 participants