This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho][test] Remove redundant -lSystem flags
Needs ReviewPublic

Authored by int3 on Dec 23 2022, 8:35 AM.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

D135193: [lld-macho][test] Add -lSystem to all lld invocations means that we can clean up the -lSystem flags in each
individual test.

Diff Detail

Event Timeline

int3 created this revision.Dec 23 2022, 8:35 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptDec 23 2022, 8:35 AM
int3 requested review of this revision.Dec 23 2022, 8:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 23 2022, 8:35 AM
thakis added a subscriber: thakis.Dec 23 2022, 9:29 AM

I'm kind of against this change: I often change %lld to ld in tests locally to compare behavior to ld64. This makes that impossible, without a big upside in return.

(I kind of wish we would've just added explicit -lSystem flags in that other patch instead of changing the substitution as well, for the same reason, and for "explicit is better than implicit")

int3 added a comment.Dec 23 2022, 11:45 AM

I often change %lld to ld in tests locally to compare behavior to ld64.

I do something similar, except that I change it in lit.local.cfg, so all the flags carry over... would that work for you?

With that approach, it's not possible to duplicate a run line and make one run line run ld64 and the other old, is it?

Also, imho it's nice if the run line looks similar to what actually runs, without lots of possibly surprising implicit additions.

int3 added a comment.Dec 29 2022, 12:36 PM

With that approach, it's not possible to duplicate a run line and make one run line run ld64 and the other old, is it?

Yeah that's a limitation. What I usually do is to run the test once with -a, copy the LLD invocation of interest, and use that to invoke ld64.

Also, imho it's nice if the run line looks similar to what actually runs, without lots of possibly surprising implicit additions.

I can see the value of that, but I also think that there are so many flags that it is sometimes hard to figure out what the important features being tested are. Factoring out the boilerplate helps with that. (In fact I don't think it helps enough... the lit test system seems rather limited in this aspect)