<div dir="ltr">Hi All,<div><br></div><div>I managed to fix all the patches and make each of them be able to build. More specifically, patch 3 ~ 6.</div><div>Here is the full list of the patches:</div><div>1. <a href="https://reviews.llvm.org/D88385">https://reviews.llvm.org/D88385</a> </div><div>2. <a href="https://reviews.llvm.org/D88386">https://reviews.llvm.org/D88386</a></div><div>3. <a href="https://reviews.llvm.org/D88389">https://reviews.llvm.org/D88389</a></div><div>4. <a href="https://reviews.llvm.org/D88390">https://reviews.llvm.org/D88390</a></div><div>5. <a href="https://reviews.llvm.org/D88391">https://reviews.llvm.org/D88391</a></div><div>6. <a href="https://reviews.llvm.org/D88392">https://reviews.llvm.org/D88392</a></div><div>7. <a href="https://reviews.llvm.org/D88393">https://reviews.llvm.org/D88393</a></div><div>8. <a href="https://reviews.llvm.org/D88394">https://reviews.llvm.org/D88394</a></div><div><br></div><div>Now while I'm addressing the issues from the mailing list and the Phabricator, I'm wondering if there is any good suggestion on the reviewers?</div><div><br></div><div>Thank you</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Sep 28, 2020 at 2:39 AM Renato Golin <<a href="mailto:rengolin@gmail.com">rengolin@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Mon, 28 Sep 2020 at 06:44, Min-Yih Hsu via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>As Nicloas mentioned, you need to compile all 8 patches as a whole. Which I think is not ideal from the point of reviewers. So I'm wondering what is the suggestion on splitting a new target backend?</div></div></blockquote><div><br></div><div>This is problematic for regression testing and bisecting to find a broken commit. The guideline is to split the patch into multiple commits, but that each commit builds on their own.</div><div><br></div><div>They don't need to have tests in between and be fully functional, but they need to compile and not to break anything.</div><div><br></div><div>Usually (and very generally) people break new targets into a few steps:</div><div> 1. Adds the new directory, base table-gen files, hooks on CMake, etc.</div><div> 2. Adds target description in table-gen (registers, etc) and operations. [this could be split in two if large]</div><div> 3. Adds codegen hooks from MIR and gets some initial "hello world" compiled, add tests for the features added</div><div><br></div><div>Steps 1 and 2 should not break anything else. Step 3's tests should all pass and not break other tests. There could be more commits than 1 per step, but not breaking build/tests nor depending on future patches.<br></div><div><br></div><div><div>You should also have a buildbot with the target enabled (there's a CMake flag for that) to show that it's green most of the time.</div><div><br></div><div>Optional but nice, you could have some kind of testing on "hardware" (which could be an emulator). Does QEMU have user emulation for 68k?</div><div><br></div><div>You don't need to add all hardware features nor all operations in the first patch-set, but it's nice if the first wave can compile a hello world program. </div><div><br></div><div></div><div>That's where the plan comes in. We usually ask the new community to provide a roadmap of what features will come in which order, so that we can help with the plan and be prepared to review the code. It also gives us more confidence that the community is serious about adding a production target, not just a toy target.</div><div><br></div><div>cheers,</div><div>--renato</div><div></div></div></div></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Min-Yih Hsu</div><div>Ph.D Student in ICS Department, University of California, Irvine (UCI).<br></div></div></div>