And this one, too: https://reviews.llvm.org/D95030

This looks good to me, thanks!

There are still some comments on the remaining patches in this set that need addressing before all of them can be committed as a whole.

Actually, this is mentioned in a different document: https://www.llvm.org/docs/Phabricator.html#committing-a-change

In D88391#2610678, @RKSimon wrote:

Kudos to everyone involved.

Some comments, mostly on style, but this looks like a fair skeleton lowering to me.

+1 to @myhsu's points, plus a few comments, but otherwise, is looking good.

This looks good to me. @MaskRay any comments?

In D88391#2565043, @glaubitz wrote:

Any chance we can get this reviewed?

In D96632#2563111, @DavidSpickett wrote:

A8.1.3 Instruction encodings

For example, the assembler
syntax documented for the 32-bit Thumb AND (register) encoding includes the .W qualifier to ensure that the
32-bit encoding is selected even for the small proportion of operand combinations for which the 16-bit
encoding is also available.

Which is not a great example because when I tested this with gcc, it would prefer 16 bit encodings in most situations
that I could come up with. However, AND has conditions for being in or out of an IT block so that might be higher priority than any .w encoding.

In D95586#2540002, @nickdesaulniers wrote:

GNU objdump only warns with <UNPREDICTABLE> for the second form (which is the form this patch is concerned with). That seems like good justification for removing the test from the kernel.

In D95386#2537500, @stella.stamenova wrote:

It looks like this broke the windows mlir bot. Please revert it or fix it ASAP:

http://lab.llvm.org:8011/#/builders/13/builds/4013

In D95386#2537038, @mehdi_amini wrote:

LG, my previous comment was more aspirational, I didn't intend to hold this revision :)

In D95386#2536963, @mjp41 wrote:

@rengolin, can you merge as is?

In D88390#2531077, @glaubitz wrote:

It's happening what I was hoping and also expecting - external people just coming by and sending such improvements because they love the retro architecture and enjoy working on LLVM.

My reading of the manual says this change is correct. All the exceptions and deprecations in MOVs and LSL seem to not be in ARM mode with the S flag set and without the PC in Rm (source register).

In D95586#2531012, @nickdesaulniers wrote:

Then why does llvm/test/MC/Disassembler/ARM/unpredictable-LSL-regform.txt test LSL, and not ARS, LSR, or ROR? (There are other tests llvm/test/MC/Disassembler/ARM/unpredictable-, but there don't seem to be tests for those).

In D88390#2529089, @myhsu wrote:

We don't have AsmParser and disassembler right now. We're using MIR as input to test our integrated assembler, so you're right, we can't test MC layer without CodeGen. But why would that will be a hard blocker?

ARS, LSR, ROR have the following warning on the ARM ARM:

In D88391#2526093, @jrtc27 wrote:

Yes and no. I had a bunch of comments on many of them and will need to make another pass over them to see if there's anything I've missed (or that's been introduced). All accepted means is that someone in the community thinks it's fine and nobody's explicitly rejected it (which I could have done, but I don't like to use the request changes feature as having big red marks over entire patch series can be demoralising). Certainly shouldn't be landed as soon as everything has an approval, rather that should be the point to request that people who have given feedback take another look over the series if they want to.

In D95386#2525931, @mehdi_amini wrote:

This patch is fine, but I'd still be interested to understand if this could be made to show up during check-mlir with one of the Cmake configuration that can run on Windows?

In D95386#2522648, @mjp41 wrote:

@ezhulenev I think I can see what the fix should be. Does the check-mlir cover this example? Or is there additional testing I should do?

This is the error:

In D93798#2518757, @zixuan-wu wrote:

anymore comments or LGTM?

Overall, I think this is great. It answers all questions I've seen posed on the RFCs and is well aligned with the tiers policy, in text and intent.

In D93798#2509226, @zixuan-wu wrote:

Yes, a first batch of patches to reach the point that it can generate first ALU asm codegen. In all, I propose following patches to construct the first batch.

https://reviews.llvm.org/D93798
https://reviews.llvm.org/D94007
https://reviews.llvm.org/D95029
https://reviews.llvm.org/D95030

Hi Zixuan,

In D88385#2506204, @RKSimon wrote:

The plan is for the entire patch series to be accepted before any are pushed.

In D88385#2502299, @myhsu wrote:

Absolutely. I agree this problem should be solve before graduate from experimental target. I will create these bugs soon

In D88385#2501781, @jrtc27 wrote:

That sounds reasonable (but +1 to @RKSimon 's comment about reviewing all this "special sauce" complexity before graduating from experimental)

In D88385#2498908, @jrtc27 wrote:

Yeah my concern is not about m68k doing weird things, it's about the code that's added in the target-independent places to support m68k that I am most concerned with, as it affects every target regardless of whether m68k is enabled.

In D94451#2491668, @tstellar wrote:

If this pitch is accepted, then what are the next steps required in order for bazel or another build system to be added to tree? For example, can they automatically be added if they meet the support policy requirements or does there need to be some kind of community ack for? I think the pitch needs to be more clear about what this process is.

Also, please use diff -U9999 as documented: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

In D88385#2476435, @RKSimon wrote:

Why would refactoring TSFlags into a int-ptr union just for M68k be better than adding the CodeBeads functionality just for M68k?

In D92748#2478802, @kaz7 wrote:

Thank you for the response. Your description is correct. That is what I tried to describe. Thanks.

In D92748#2454376, @kaz7 wrote:

@compnerd , I have a question. I'm thinking that you might misunderstand my explanations. VE is almost merged. I can compile llvm-project using already upstreamed llvm/clang. Now, in order to continue the upstreaming, I need crtbegin.o and crtend.o in compiler-rt. It is a reason why I submit this patch. Didn't you misunderstand the situation? Thanks!

In D93798#2475498, @zixuan-wu wrote:

I think it's not worthwhile to spend too much time to discuss this open topic. Let's come back to review this and following patches, please. Thank you! I can make few diff files based on this one.

In D93798#2474617, @zixuan-wu wrote:

I think it's early-refactoring issue. We should assume the current patch is good in current codebase context, or we will re-review many times.

In D93798#2473365, @zixuan-wu wrote:

I don't think too many patches on the fly is good to upstream because they influence each other and need change on the way of review.

LGTM, thanks!

This LGTM now, thanks!

LGTM too, thanks!

Looks like a standard mock implementation, basic infrastructure. I don't see anything wrong with it but I've also been far from this area for too long, so I'll let other people review it properly and approve.

LGTM too, thanks!

This looks good to me but I'll let other people review again and approve. Thanks!

@craig.topper are you happy with the changes?

In D88385#2442226, @RKSimon wrote:

Does anyone have any more comments? Otherwise I think we can provisionally accept this.

In D88391#2441942, @zixuan-wu wrote:

Hmm, I am just curious about that whether GlobalISEL is mature enough to use as default instruction selection method without implementing SelectionDAG. As a newer backend, I really would like to drop legacy stuff at start if it works.

Thanks for the changes. This looks good to me but I'll let others check again and approve.

I can't possibly comment on the ISA implementation, but this looks like a standard table-gen set of files to me. They also don't affect anything else outside of CSKY, so LGTM.

You should at least get @craig.topper's feedback, given this is a significant change in the x86 backend.

In D88389#2401213, @myhsu wrote:

Oh, from the discussion on mailing list I thought this is a requirement.

In D88389#2399389, @RKSimon wrote:

Is it worth trying to avoid adding/deleting the *Dummy* files in the patch series? @rengolin any thoughts ?

Nice set of tests, well separated, not a lot of pattern-match on the FileCheck side, which is good, asm and object emission.

I only had a very quick overview and this looks fine.

A few nits, but this is looking good.

Hi @fpetrogalli, the document is so dense that it took me a while to check the macros and I was still wrong.

It's looking a bit redundant to me, but I wanted to make sure we have the link on all three sections in case people are given a link (or navigate from the table-of contents) and don't see the sections above.

Sorry, forgot the review text on the commit message:
Closed by 25ba6b2bcd1

Thanks Chris! Will do.

In D90761#2380282, @mehdi_amini wrote:

LGTM from me on the principle, I didn't get into detailed wordsmith. Overall I would have been more concise and lay down the general principle rather than trying to prescribe this level of details (like the "Reinstatement" section for example), I believe that's also more in line with how we've been operating / documenting things in the past, but I don't really mind it either.
I suspect we can incrementally fix/improve anything as we go as well.

• Include a short phrase on inclusion policy, as requested.
• Fixed some typos

Simple and straightforward, with documentation! :)

Nice change, simpler to read and easier to get right. LGTM, thanks!

Sorry, s/George/Geoffrey/. :S

• Adding inclusion policy section
• Reword phrase in deprecation policy

In D90761#2374520, @GMNGeoffrey wrote:

Overall LGTM, with some wording nits. One thing I think is missing is guidance for how a peripheral component gets added. Experimental targets already have their own documentation, and my impression is that editor bindings are just a matter of sending a patch (which seems reasonable to me). What about something like:

To add a new peripheral component not covered under an existing policy, send an RFC to the appropriate dev list proposing its addition and explaining how it will meet the support requirements listed here.

Rewriting some confusing paragraphs.

Updating simple suggestions / fixups.

• Adding page link to "Getting Involved" page.
• Fixing some formatting errors (Sphinx reports no warnings on my box).