Page MenuHomePhabricator

Documentation for the process of adding new targets
ClosedPublic

Authored by rengolin on Dec 28 2021, 4:34 AM.

Details

Summary

Plenty of new targets nowadays and I found myself repeating the same
thing over and over, so this is more or less what we said over the last
few years, but condensed in an ordered fashion and easy to digest.

This does not change any of the recommendations, only documents what we
have been saying for years. Some of the new targets have done slightly
different processes because we didn't have this document, so I want to
make sure new targets don't suffer from this problem.

Copying some target owners that I helped upstream to make sure I'm not
missing anything. Feel free to copy more people if needed. Also copying
some active developers that have reviewed documentation changes recently.

Diff Detail

Unit TestsFailed

TimeTest
50 msx64 debian > LLVM.Bindings/Go::go.test
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llvm-go go=/usr/bin/go test llvm.org/llvm/bindings/go/llvm
2,660 msx64 debian > libFuzzer.libFuzzer::fuzzer-finalstats.test
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/fuzzer -m64 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/fuzzer/SimpleTest.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/fuzzer-finalstats.test.tmp-SimpleTest

Event Timeline

rengolin requested review of this revision.Dec 28 2021, 4:34 AM
rengolin created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptDec 28 2021, 4:34 AM
nickdesaulniers accepted this revision.Dec 28 2021, 10:41 AM

LGTM; thanks for adding this. Reading through https://llvm.org/docs/DeveloperPolicy.html#adding-a-new-target, consider adding these numbered points under the bulleted list of points under The basic rules for a back-end to be upstreamed in experimental mode are:.

llvm/docs/DeveloperPolicy.rst
820

s/that/then/

This revision is now accepted and ready to land.Dec 28 2021, 10:41 AM

LGTM; thanks for adding this. Reading through https://llvm.org/docs/DeveloperPolicy.html#adding-a-new-target, consider adding these numbered points under the bulleted list of points under The basic rules for a back-end to be upstreamed in experimental mode are:.

IIRC it was numbered on the original patch but we changed to bullets. I'll leave this to another patch, as I'm not sure they should be numbered either.

llvm/docs/DeveloperPolicy.rst
820

I changed to "only after", which I think is what was more confusing.

rengolin closed this revision.EditedDec 28 2021, 1:01 PM

Thanks!

zixuan-wu removed a subscriber: Zeson.Dec 28 2021, 6:47 PM
zixuan-wu added a subscriber: zixuan-wu.

Thanks for adding the doc!

I don't have any concern with what you wrote, but from a "process" point of view: it'd be great to let this kind of things (important documentation) up for review a bit longer than a day, especially in a week of the year where most people may not be online.