[PATCH] D28079: [GlobalISel] Add support for indirectbr

[Quentin Colombet via llvm-commits]

> On Jan 5, 2017, at 6:51 AM, Kristof Beyls via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> kristof.beyls marked 2 inline comments as done.
> kristof.beyls added a comment.
> 
> In https://reviews.llvm.org/D28079#636013, @qcolombet wrote:
> 
>> Hi,
>> 
>> Regarding (G)MIR vs. (T)MIR or whatever, I would stick to MIR for intermediate MachineInstr representation without generic opcode and Generic MachineInstr (or GMIR) when generic opcode are present. The story behind (G)MIR is that this is a*always* a target specific representation (because MachineInstr are always bound to a target) but can optionally (thus parenthesis) contain generic opcodes.
>> 
>> Therefore the difference between (G)MIR and MIR is the latter cannot have generic opcodes whereas the former can, but again in both cases the representation is target depend.
>> 
>> The bottom line is please don't add another naming convention.
>> 
>> Cheers,
>> -Quentin
> 
> 
> Having the convention that MIR represents MachineInstr representation that cannot have generic opcodes is fine for me - I was just wondering what the convention was. Thanks for explaining, Quentin.
> 
> I've also addressed all your other remarks; thanks for the feedback!
> 
> 
> 
> ================
> Comment at: test/CodeGen/X86/implicit-null-checks.mir:322
> # CHECK:  %rbx = MOV64rr %rdx
> -# CHECK-NEXT:  %rdi = FAULTING_LOAD_OP %bb.3.is_null, 260, killed %rbx, killed %rdi, 1, _, 0, _, implicit-def dead %eflags :: (load 4 from %ir.x)
> +# CHECK-NEXT:  %rdi = FAULTING_LOAD_OP %bb.3.is_null, {{[0-9]+}}, killed %rbx, killed %rdi, 1, _, 0, _, implicit-def dead %eflags :: (load 4 from %ir.x)
> 
> ----------------
> qcolombet wrote:
>> This is an unrelated change, right?
> The hard-coded 260 in the test seems to be a specific TargetOpcode value. Since this patch adds a new TargetOpcode (G_BRINDIRECT), the value changes, but obviously only if you build LLVM with GlobalISel enabled.

Ah right, sounds good then!

> 
> I followed Tim's lead from
> http://llvm.org/viewvc/llvm-project?view=revision&revision=274774 to fix the test in the same way as another test in this same file that had the same issue at that time. I've believed Tim's word that these tests don't care about the internal opcode number.
> 
> 
> https://reviews.llvm.org/D28079
> 
> 
>