LGTM

LGTM

I'm not convinced you really need the FixedLenDecoderEmitter.cpp changes. PPCInstrFormats.td has bits<64> Inst in the I2 class, but PPCDisassembler.cpp has InsnType as being uint32_t . InsnType needs to be able to hold your biggest instruction for the generated code to work correctly. I believe that if you changed:

uint32_t Inst = ...;

in PPCDisassembler::getInstruction(), to:

uint64_t Inst = ...;

then you will no longer need to change FixedLenDecoderEmitter.cpp as you'll be able to use shifts of up to 63.

Fix the remaining small issues

+Testcase

isInteger -> isIntegerLike and comment on pointers needing to use isIntegerLike

I see you've added me as a reviewer but I haven't worked on MIPS for a few years so I'm not really familiar with the current state of their linkers and binaries. AFAIK @atanasyan is still actively working on MIPS so hopefully he can take a look or bring in a colleague who can.
FWIW, this sounds like a good change and the code looks good to me but I think you should get another LGTM from someone more up to date on MIPS.

Thanks everyone. Landed in a5230ac10b0

Add a bit about asking someone to commit for you so that we can document
providing the information needed for the Author property

Small changes for review comments

LGTM

In D69836#1811303, @thakis wrote:

Temporarily reverted in 71d64f72f934631aa2f for now.

In D69152#1808445, @thakis wrote:

This broke tests on Windows: http://45.33.8.238/win/5239/step_11.txt

Probably the "%p doesn't include 0x on Windows" thing again.

In D72309#1807593, @arichardson wrote:

Could we invert the boolean flag to be isFloat? I fear that calling it isInteger, will lead the the same problems that I tried to fix in that commit (calling .isInteger() returns false for pointers).

Corrected patch

LGTM. Thanks!

LGTM too. IIRC, the ability to have multiple trees came after the original implementation so I think we just had a direct conversion.

LGTM

Rebase

Rebase

rebase

Landed in rG520e3d66e7257c77f1226185504bbe1cb90afcfa (and re-committed in rG7ea2e5195a82bd9370d0b4362512b67c8b2cde80

Update before commit (and because arcanist keeps pulling down the wrong commit message)

I've pushed 7ea2e5195a8 which should address these three issues

In D69077#1788377, @uweigand wrote:

The new test case causes build bot failures (hidden by another failure that was already present):
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/28934/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3Aparse-match-pattern.td

/home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/llvm/test/TableGen/GICombinerEmitter/parse-match-pattern.td:212:16: error: CHECK-NEXT: expected string not found in input
// CHECK-NEXT: 0:$<def>, 1:mi0, 2:mi1
               ^
<stdin>:129:16: note: scanning from here
 0:$<def>, 1:mi0, 2:mi1
               ^
<stdin>:130:2: note: possible intended match here
 0:$<def>, 1:mi
 ^

I'm just going through the remaining issues from Matt and I don't think there's anything that should block committing this. AFAICT there's only one issue still open and it's the one about reusing the static printIRValueReference() in MachineOperand.cpp causing one function of MIRFormatter to be implemented in that file as well.

In rG327894859cc41c1730807f8a179aa880203262f5#856678, @dsanders wrote:

In rG327894859cc41c1730807f8a179aa880203262f5#856567, @kadircet wrote:

Could you also update https://llvm.org/docs/CommandGuide/lit.html#pre-defined-substitutions to mention this new extension?

Sure

In rG327894859cc41c1730807f8a179aa880203262f5#856567, @kadircet wrote:

Could you also update https://llvm.org/docs/CommandGuide/lit.html#pre-defined-substitutions to mention this new extension?

LGTM from me.

Thanks for doing this. It should make a lot of things easier to read.

In D69734#1734315, @arsenm wrote:

In D69734#1734287, @dsanders wrote:

Thanks.

I realized I missed a couple things in this patch this morning:

• Filenames containing 'gep'
• Functions named buildGep() and similar I'll fix both of this this before pushing

If you're fixing names of instructions, that buildUndef doesn't match the G_IMPLICIT_DEF name has bothered me. I'm not sure which should be renamed

... this this ...

I realized I missed a couple things in this patch this morning:

• Filenames containing 'gep'
• Functions named buildGep() and similar

I'll fix both of this this before pushing