diffkemp icon indicating copy to clipboard operation
diffkemp copied to clipboard

Reggresion tests do not rebuild llvm

Open PLukas2018 opened this issue 11 months ago • 3 comments

When I tried to change the flags used for the compilation of C files to LLVM (specifically from -O1 to -O0 -Xclang -disable-O0-optnone), I noticed if the LLVM files already exist they are not rebuilt so the tests can give a wrong result (from before the changes). To overcome this it is necessary to manually delete *.ll files from kernel directory.

This could be fixed, but I think that changes in the build commands are very rarely done, so it does not probably make sense to do it because it would prolong local testing when only changes in compare phase would be done -- all the necessary LLVM files would be always rebuilt which would take time. I just wanted to inform about this behaviour. Btw. the CI gives the correct result (I think that the LLVM IR files are not cached in CI).

Note, the reason, why the LLVM IR are not rebuild is here https://github.com/viktormalik/diffkemp/blob/789ac18c2c6864b003eac2d77d155cdf87da0a72/diffkemp/llvm_ir/kernel_llvm_source_builder.py#L569-L570

PLukas2018 avatar Mar 08 '24 08:03 PLukas2018

Yes, I know about this issue and it's as you say - changes in the build command are occasional so I prefer not to waste test time on building. The only way to improve this is to introduce a flag to pytest test (if that's even possible) which will force the rebuild (instead of needing to delete *.ll files manually).

Still, it's good to keep this issue open to have the behavior documented.

viktormalik avatar Mar 08 '24 10:03 viktormalik

It is possible to introduce custom flag to pytest and then add some logic around it, it is docuemented here.

DanielKriz avatar Jul 09 '24 07:07 DanielKriz

It is possible to introduce custom flag to pytest and then add some logic around it, it is docuemented here.

Thanks for the link. Let's first resolve the test refactoring (#351) and then we can try to address this.

viktormalik avatar Jul 09 '24 07:07 viktormalik