[AMDGPU] Test codegen'ing True16 additions.
[llvm-project.git] / llvm / docs / MyFirstTypoFix.rst
blob27324049af33039fafdd330bd512bafdf28386d2
1 ==============
2 MyFirstTypoFix
3 ==============
5 .. contents::
6    :local:
8 Introduction
9 ============
11 This tutorial will guide you through the process of making a change to
12 LLVM, and contributing it back to the LLVM project. We'll be making a
13 change to Clang, but the steps for other parts of LLVM are the same.
14 Even though the change we'll be making is simple, we're going to cover
15 steps like building LLVM, running the tests, and code review. This is
16 good practice, and you'll be prepared for making larger changes.
18 We'll assume you:
20 -  know how to use an editor,
22 -  have basic C++ knowledge,
24 -  know how to install software on your system,
26 -  are comfortable with the command line,
28 -  have basic knowledge of git.
31 The change we're making
32 -----------------------
34 Clang has a warning for infinite recursion:
36 .. code:: console
38    $ echo "void foo() { foo(); }" > ~/test.cc
39    $ clang -c -Wall ~/test.cc
40    input.cc:1:14: warning: all paths through this function will call
41    itself [-Winfinite-recursion]
43 This is clear enough, but not exactly catchy. Let's improve the wording
44 a little:
46 .. code:: console
48    input.cc:1:14: warning: to understand recursion, you must first
49    understand recursion [-Winfinite-recursion]
52 Dependencies
53 ------------
55 We're going to need some tools:
57 -  git: to check out the LLVM source code,
59 -  a C++ compiler: to compile LLVM source code. You'll want `a recent
60    version <https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library>`__
61    of Clang, GCC, or Visual Studio.
63 -  CMake: used to configure how LLVM should be built on your system,
65 -  ninja: runs the C++ compiler to (re)build specific parts of LLVM,
67 -  python: to run the LLVM tests,
69 -  arcanist: for uploading changes for review,
71 As an example, on Ubuntu:
73 .. code:: console
75    $ sudo apt-get install git clang cmake ninja-build python arcanist
78 Building LLVM
79 =============
82 Checkout
83 --------
85 The source code is stored `on
86 Github <https://github.com/llvm/llvm-project>`__ in one large repository
87 ("the monorepo").
89 It may take a while to download!
91 .. code:: console
93    $ git clone https://github.com/llvm/llvm-project.git
95 This will create a directory "llvm-project" with all of the source
96 code.(Checking out anonymously is OK - pushing commits uses a different
97 mechanism, as we'll see later)
99 Configure your workspace
100 ------------------------
102 Before we can build the code, we must configure exactly how to build it
103 by running CMake. CMake combines information from three sources:
105 -  explicit choices you make (is this a debug build?)
107 -  settings detected from your system (where are libraries installed?)
109 -  project structure (which files are part of 'clang'?)
111 First, create a directory to build in. Usually, this is
112 llvm-project/build.
114 .. code:: console
116    $ mkdir llvm-project/build
117    $ cd llvm-project/build
119 Now, run CMake:
121 .. code:: console
123    $ cmake -G Ninja ../llvm -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang
125 If all goes well, you'll see a lot of "performing test" lines, and
126 finally:
128 .. code:: console
130    Configuring done
131    Generating done
132    Build files have been written to: /path/llvm-project/build
134 And you should see a build.ninja file.
136 Let's break down that last command a little:
138 -  **-G Ninja**: we're going to use ninja to build; please create
139    build.ninja
141 -  **../llvm**: this is the path to the source of the "main" LLVM
142    project
144 -  The two **-D** flags set CMake variables, which override
145    CMake/project defaults:
147 -  **CMAKE\ BUILD\ TYPE=Release**: build in optimized mode, which is
148    (surprisingly) the fastest option.
150    If you want to run under a debugger, you should use the default Debug
151    (which is totally unoptimized, and will lead to >10x slower test
152    runs) or RelWithDebInfo which is a halfway point.
153    **CMAKE\ BUILD\ TYPE** affects code generation only, assertions are
154    on by default regardless! **LLVM\ ENABLE\ ASSERTIONS=Off** disables
155    them.
157 -  **LLVM\ ENABLE\ PROJECTS=clang** : this lists the LLVM subprojects
158    you are interested in building, in addition to LLVM itself. Multiple
159    projects can be listed, separated by semicolons, such as "clang;
160    lldb".In this example, we'll be making a change to Clang, so we
161    should build it.
163 Finally, create a symlink (or a copy) of
164 llvm-project/build/compile-commands.json into llvm-project/:
166 .. code:: console
168    $ ln -s build/compile_commands.json ../
170 (This isn't strictly necessary for building and testing, but allows
171 tools like clang-tidy, clang-query, and clangd to work in your source
172 tree).
175 Build and test
176 --------------
178 Finally, we can build the code! It's important to do this first, to
179 ensure we're in a good state before making changes. But what to build?
180 In ninja, you specify a **target**. If we just want to build the clang
181 binary, our target name is "clang" and we run:
183 .. code:: console
185    $ ninja clang
187 The first time we build will be very slow - Clang + LLVM is a lot of
188 code. But incremental builds are fast: ninja will only rebuild the parts
189 that have changed. When it finally finishes you should have a working
190 clang binary. Try running:
192 .. code:: console
194    $ bin/clang --version
196 There's also a target for building and running all the clang tests:
198 .. code:: console
200    $ ninja check-clang
202 This is a common pattern in LLVM: check-llvm is all the checks for core,
203 other projects have targets like check-lldb.
206 Making changes
207 ==============
210 Edit
211 ----
213 We need to find the file containing the error message.
215 .. code:: console
217    $ git grep "all paths through this function" ..
218    ../clang/include/clang/Basic/DiagnosticSemaKinds.td:  "all paths through this function will call itself">,
220 The string that appears in DiagnosticSemaKinds.td is the one that is
221 printed by Clang. \*.td files define tables - in this case it's a list
222 of warnings and errors clang can emit and their messages. Let's update
223 the message in your favorite editor:
225 .. code:: console
227    $ vi ../clang/include/clang/Basic/DiagnosticSemaKinds.td
229 Find the message (it should be under
230 warn\ *infinite*\ recursive_function)Change the message to "in order to
231 understand recursion, you must first understand recursion".
234 Test again
235 ----------
237 To verify our change, we can build clang and manually check that it
238 works.
240 .. code:: console
242    $ ninja clang
243    $ bin/clang -Wall ~/test.cc
244    /path/test.cc:1:124: warning: in order to understand recursion, you must
245    first understand recursion [-Winfinite-recursion]
247 We should also run the tests to make sure we didn't break something.
249 .. code:: console
251    $ ninja check-clang
253 Notice that it is much faster to build this time, but the tests take
254 just as long to run. Ninja doesn't know which tests might be affected,
255 so it runs them all.
257 .. code:: console
259    ********************
260    Testing Time: 408.84s
261    ********************
262    Failing Tests (1):
263        Clang :: SemaCXX/warn-infinite-recursion.cpp
265 Well, that makes senseā€¦ and the test output suggests it's looking for
266 the old string "call itself" and finding our new message instead.
267 Note that more tests may fail in a similar way as new tests are
268 added time to time.
270 Let's fix it by updating the expectation in the test.
272 .. code:: console
274    $ vi ../clang/test/SemaCXX/warn-infinite-recursion.cpp
276 Everywhere we see `// expected-warning{{call itself}}` (or something similar
277 from the original warning text), let's replace it with
278 `// expected-warning{{to understand recursion}}`.
280 Now we could run **all** the tests again, but this is a slow way to
281 iterate on a change! Instead, let's find a way to re-run just the
282 specific test. There are two main types of tests in LLVM:
284 -  **lit tests** (e.g. SemaCXX/warn-infinite-recursion.cpp).
286 These are fancy shell scripts that run command-line tools and verify the
287 output. They live in files like
288 clang/**test**/FixIt/dereference-addressof.c. Re-run like this:
290 .. code:: console
292    $ bin/llvm-lit -v ../clang/test/SemaCXX/warn-infinite-recursion.cpp
294 -  **unit tests** (e.g. ToolingTests/ReplacementTest.CanDeleteAllText)
296 These are C++ programs that call LLVM functions and verify the results.
297 They live in suites like ToolingTests. Re-run like this:
299 .. code:: console
301    $ ninja ToolingTests && tools/clang/unittests/Tooling/ToolingTests
302    --gtest_filter=ReplacementTest.CanDeleteAllText
305 Commit locally
306 --------------
308 We'll save the change to a local git branch. This lets us work on other
309 things while the change is being reviewed. Changes should have a
310 description, to explain to reviewers and future readers of the code why
311 the change was made.
313 .. code:: console
315    $ git checkout -b myfirstpatch
316    $ git commit -am "[Diagnostic] Clarify -Winfinite-recursion message"
318 Now we're ready to send this change out into the world! By the way,
319 There is an unwritten convention of using tag for your commit. Tags
320 usually represent modules that you intend to modify. If you don't know
321 the tags for your modules, you can look at the commit history :
322 https://github.com/llvm/llvm-project/commits/main.
325 Code review
326 ===========
329 Finding a reviewer
330 ------------------
332 Changes can be reviewed by anyone in the LLVM community who has commit
333 access.For larger and more complicated changes, it's important that the
334 reviewer has experience with the area of LLVM and knows the design goals
335 well. The author of a change will often assign a specific reviewer (git
336 blame and git log can be useful to find one).
338 As our change is fairly simple, we'll add the cfe-commits mailing list
339 as a subscriber; anyone who works on clang can likely pick up the
340 review. (For changes outside clang, llvm-commits is the usual list. See
341 `http://lists.llvm.org/ <http://lists.llvm.org/mailman/listinfo>`__ for
342 all the \*-commits mailing lists).
345 Uploading a change for review
346 -----------------------------
348 .. warning::
350   Phabricator is deprecated and will be switched to read-only mode in October
351   2023. For new code contributions use :ref:`GitHub Pull Requests <github-reviews>`.
352   This section contains old information that needs to be updated.
354 LLVM code reviews happen at https://reviews.llvm.org. The web interface
355 is called Phabricator, and the code review part is Differential. You
356 should create a user account there for reviews (click "Log In" and then
357 "Register new account").
359 Now you can upload your change for review:
361 .. code:: console
363    $ arc diff HEAD^
365 This creates a review for your change, comparing your current commit
366 with the previous commit. You will be prompted to fill in the review
367 details. Your commit message is already there, so just add cfe-commits
368 under the "subscribers" section. It should print a code review URL:
369 https://reviews.llvm.org/D58291 You can always find your active reviews
370 on Phabricator under "My activity".
373 Review process
374 --------------
376 When you upload a change for review, an email is sent to you, the
377 cfe-commits list, and anyone else subscribed to these kinds of changes.
378 Within a few days, someone should start the review. They may add
379 themselves as a reviewer, or simply start leaving comments. You'll get
380 another email any time the review is updated. The details are in the
381 `https://llvm.org/docs/CodeReview/ <https://llvm.org/docs/CodeReview.html>`__.
384 Comments
385 ~~~~~~~~
387 The reviewer can leave comments on the change, and you can reply. Some
388 comments are attached to specific lines, and appear interleaved with the
389 code. You can either reply to these, or address them and mark them as
390 "done". Note that in-line replies are **not** sent straight away! They
391 become "draft" comments and you must click "Submit" at the bottom of the
392 page.
395 Updating your change
396 ~~~~~~~~~~~~~~~~~~~~
398 If you make changes in response to a reviewer's comments, simply run
400 .. code:: console
402    $ arc diff
404 again to update the change and notify the reviewer. Typically this is a
405 good time to send any draft comments as well.
408 Accepting a revision
409 ~~~~~~~~~~~~~~~~~~~~
411 When the reviewer is happy with the change, they will **Accept** the
412 revision. They may leave some more minor comments that you should
413 address, but at this point the review is complete. It's time to get it
414 committed!
417 Commit by proxy
418 ---------------
420 As this is your first change, you won't have access to commit it
421 yourself yet. The reviewer **doesn't know this**, so you need to tell
422 them! Leave a message on the review like:
424    Thanks @somellvmdev. I don't have commit access, can you land this
425    patch for me? Please use "My Name my@email" to commit the change.
427 The review will be updated when the change is committed.
430 Review expectations
431 -------------------
433 In order to make LLVM a long-term sustainable effort, code needs to be
434 maintainable and well tested. Code reviews help to achieve that goal.
435 Especially for new contributors, that often means many rounds of reviews
436 and push-back on design decisions that do not fit well within the
437 overall architecture of the project.
439 For your first patches, this means:
441 -  be kind, and expect reviewers to be kind in return - LLVM has a `Code
442    of Conduct <https://llvm.org/docs/CodeOfConduct.html>`__;
444 -  be patient - understanding how a new feature fits into the
445    architecture of the project is often a time consuming effort, and
446    people have to juggle this with other responsibilities in their
447    lives; **ping the review once a week** when there is no response;
449 -  if you can't agree, generally the best way is to do what the reviewer
450    asks; we optimize for readability of the code, which the reviewer is
451    in a better position to judge; if this feels like it's not the right
452    option, you can contact the cfe-dev mailing list to get more feedback
453    on the direction;
456 Commit access
457 =============
459 Once you've contributed a handful of patches to LLVM, start to think
460 about getting commit access yourself. It's probably a good idea if:
462 -  you've landed 3-5 patches of larger scope than "fix a typo"
464 -  you'd be willing to review changes that are closely related to yours
466 -  you'd like to keep contributing to LLVM.
469 Getting commit access
470 ---------------------
472 LLVM uses Git for committing changes. The details are in the `developer
473 policy
474 document <https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access>`__.
477 With great power
478 ----------------
480 Actually, this would be a great time to read the rest of the `developer
481 policy <https://llvm.org/docs/DeveloperPolicy.html>`__, too. At minimum,
482 you need to be subscribed to the relevant commits list before landing
483 changes (e.g. llvm-commits@lists.llvm.org), as discussion often happens
484 there if a new patch causes problems.
487 Commit
488 ------
490 Let's say you have a change on a local git branch, reviewed and ready to
491 commit. Things to do first:
493 -  if you used multiple fine-grained commits locally, squash them into a
494    single commit. LLVM prefers commits to match the code that was
495    reviewed. (If you created one commit and then used "arc diff", you're
496    fine)
498 -  rebase your patch against the latest LLVM code. LLVM uses a linear
499    history, so everything should be based on an up-to-date origin/main.
501 .. code:: console
503    $ git pull --rebase https://github.com/llvm/llvm-project.git main
505 -  ensure the patch looks correct.
507 .. code:: console
509    $ git show
511 -  run the tests one last time, for good luck
513 At this point git show should show a single commit on top of
514 origin/main.
516 Now you can push your commit with
518 .. code:: console
520    $ git push https://github.com/llvm/llvm-project.git HEAD:main
522 You should see your change `on
523 GitHub <https://github.com/llvm/llvm-project/commits/main>`__ within
524 minutes.
527 Post-commit errors
528 ------------------
530 Once your change is submitted it will be picked up by automated build
531 bots that will build and test your patch in a variety of configurations.
533 You can see all configurations and their current state in a waterfall
534 view at http://lab.llvm.org/buildbot/#/waterfall. The waterfall view is good
535 to get a general overview over the tested configurations and to see
536 which configuration have been broken for a while.
538 The console view at http://lab.llvm.org/buildbot/#/console helps to get a
539 better understanding of the build results of a specific patch. If you
540 want to follow along how your change is affecting the build bots, **this
541 should be the first place to look at** - the colored bubbles correspond
542 to projects in the waterfall.
544 If you see a broken build, do not despair - some build bots are
545 continuously broken; if your change broke the build, you will see a red
546 bubble in the console view, while an already broken build will show an
547 orange bubble. Of course, even when the build was already broken, a new
548 change might introduce a hidden new failure.
550 | When you want to see more details how a specific build is broken,
551   click the red bubble.
552 | If post-commit error logs confuse you, do not worry too much -
553   everybody on the project is aware that this is a bit unwieldy, so
554   expect people to jump in and help you understand what's going on!
556 buildbots, overview of bots, getting error logs.
559 Reverts
560 -------
562 if in doubt, revert and re-land.
565 Conclusion
566 ==========
568 llvm is a land of contrasts.