Support Tricore compiler from Tasking #942
Conversation
louiscaron
changed the title
|
I would like to get a first review to make sure that my patch meets the implementation requirements |
louiscaron
force-pushed
the
master
branch
4 times, most recently
from
7794500
to
9851d84
Compare
|
@jrosdahl : would you mind approving the workflows on my pull request? thanks |
|
@jrosdahl : could I ask you to run the workflows again? thanks |
|
@jrosdahl : could I ask you to run the workflows again? I fixed all the issues reported by the previous run |
louiscaron
force-pushed
the
master
branch
3 times, most recently
from
affbeba
to
c93f9c3
Compare
|
@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 |
|
@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 |
|
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. |
|
@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:
Let me know if you have other ideas to get it to be merged, thanks |
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.
That could be helping with designing or thinking about #956 and #957, or helping out with other maintenance aspects of ccache. |
|
Is there anything we can still do? I merged @louiscaron changes with v4.6 release (mostly mechanistic chanes, not sure about argprocessing.cpp). |
|
Thanks @vsplesk for the merge in the dedicated branch.... |
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.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| 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
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