Page MenuHomePhabricator

[compiler-rt] Allow override of 'emulator' value from lit_config.
ClosedPublic

Authored by abidh on Jul 27 2020, 4:03 PM.

Details

Summary

Currently the 'emulator' value is fixed at build time. This patch allows changing the emulator at testing time and enables us to run the tests on different board or simulators without needing to run CMake again to change the value of emulator.

With this patch in place, the value of 'emulator' can be changed at test time from the command line like this:

$ llvm-lit --param=emulator="..."

Diff Detail

Event Timeline

abidh created this revision.Jul 27 2020, 4:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2020, 4:03 PM
Herald added subscribers: Restricted Project, dberris. · View Herald Transcript

@abidh Can you explain why this is needed? The emulator is set at CMake configure time and its value depends on the target being tested. Why do you want to override this with a value from the lit_config object?

abidh added a comment.Fri, Oct 9, 2:09 AM

@abidh Can you explain why this is needed? The emulator is set at CMake configure time and its value depends on the target being tested. Why do you want to override this with a value from the lit_config object?

Thanks for having a look. I am working with embedded targets. It helps that I can change the emulator at testing time. This allows me to run the tests on different board or simulators without needing to run cmake again to change the value of emulator. With this patch, I can change emulator value at test time like this:

$ llvm-lit --param=emulator="..."

in libc++, this thing already works and you can set your executor like this at test time
$llvm-lit --param=executor="..."

delcypher requested changes to this revision.Fri, Oct 9, 2:07 PM

Ah I see. That seems pretty reasonable. I had forgotten that lit_config.params is set from the command line. I'd suggest adding the comment I've suggested to make this clearer.
It would also be helpful to update the commit message to expand on the reason why you are doing this with the reason you just gave me in this review.

compiler-rt/test/lit.common.cfg.py
132

Maybe

# Copied from libcxx's config.py
134

Perhaps add a comment like.

# Allow overriding on the command line using --param=<name>=<val>
This revision now requires changes to proceed.Fri, Oct 9, 2:07 PM
abidh updated this revision to Diff 297486.Sun, Oct 11, 1:15 PM
abidh edited the summary of this revision. (Show Details)

Updated the comments and summary message as suggested in the review.

abidh marked 2 inline comments as done.Sun, Oct 11, 1:16 PM
This revision is now accepted and ready to land.Mon, Oct 12, 12:01 PM