Isolate shared C files to a common shared library#1693
Isolate shared C files to a common shared library#1693z5146542 wants to merge 13 commits intoherd:masterfrom
Conversation
…an error if this is not the case
diaolo01
left a comment
There was a problem hiding this comment.
Please look into getting this PR to pass make litmus-aarch64-test locally.
I have also attempted to build AArch64/A11.litmus and it fails for me. Can you please investigate.
litmus/objUtil.ml
Outdated
| do_cpy fnames (Filename.concat platform name) name ext | ||
|
|
||
| (* Copy from platform subdirectory to shared library *) | ||
| let cpy_shared_platform fnames name ext = |
There was a problem hiding this comment.
For the previous case, I agree that we can use cpy' but for cpy_platform this needs change because this method does not specify distinct src and dst arguments. I think simply removing cpy_platform and moving cpy_shared_platform to cpy_platform should solve the problem.
There was a problem hiding this comment.
I see the difference. I don't understand why there is cpy and cpy' - to me, it could have been a single function but whenever the call happened to cpy then dst=src. Am I missing anything?
That would be my preferred approach for `cpy_platform' - we can introduce a new argument that is the destination. I am open to suggestions, but let's not duplicate code.
There was a problem hiding this comment.
No, I also agree -- I think cpy can also be removed. Happy to do so if you think it should be done.
Thanks for this. This should now be fixed, and |
|
Hi @z5146542, I have failed to compile a vmsa test. On my local machine: On "chianti" (some AArch64 machine, with kvm-unit-tests installed) Could you tell me more about the motivation of this PR? |
|
Hi @maranget ,
When the output is specified to be a tarball, the ...and then run
The main motivation behind this PR is to remove unnecessary duplication of commonly used Essentially, when running |
|
Hi @z5146542. I do think the old behaviour has to be preserved: issuing I am not convinced that copying the various utility files is too high a price for having self-contained tarballs. Here are some reasons. Utility source code is shared between the tests compiled in the same batch, i.e. by the same invocation of litmus7. Even more, when using the |
|
@z5146542 I agree with @maranget and I don't see why we are making this change. I don't think that we want to have a common library across different batches of limtus7 outputs. I don't see the value in doing that. When we first discussed this, the idea was to try and move functions that now are inlined into a header that can be included and a source .c file that we can compile and link with. For example, when I do: litmus7 produces the following source files: all of them have a copy of the function
|
|
I agree with @relokin. I know of at least another example of C code that is currently inlined and that can maybe be in an utility source file: the hashtable code used in the |
This pull request now puts commonly used
.cand.hfiles in a shared library directory.Previosuly, when the C code is generated, all relevant files are created within the specified root directory:
With this change, common
.cand.hfiles are now moved to a shared library directory, andMakefileis made aware of this change. The behaviour is different depending on whether the specified output is a tarball.If the output is is not a tarball, we now have the following directories and files generated:
If the output is a tarball, the tarball will include the
shared_libdirectory and ultimately comprise the following: