[PATCH] D13593: [mips] wrong opcode for ll/sc instructions on mipsr6 when -integrated-as is used

[Daniel Sanders via llvm-commits]

dsanders added a comment.

In http://reviews.llvm.org/D13593#265806, @Jelena.Losic wrote:

> Test implementation given in the patch is able to detect situation when pre-R6 opcode is generated for sc instruction.
>  Object code is generated by llc, disassembled by llvm-objdump (command line option -mattr=+mips64r6 is used) and llvm-objdump output is searched (by using grep - it is used in Object tests) for occurrence of sc mnemonic.
>  Because -mattr=+mips64r6 option is used llvm-objdump is not able to disassemble pre-R6 instruction opcodes and reports warning instead of instruction mnemonic.


That makes sense. So long as the tablegen-erated disassembler table has the encoding correct (and we test this elsewhere) then it can tell the difference. However, we can't rely on this in the long term since the invalid opcode will be re-used at some point and we don't know what the mnemonic will be.

> Is it OK with you if I move test in CodeGen and modify it to use FileCheck instead of grep or some other approach is needed to check generated opcodes?


Moving the test to CodeGen and using FileCheck and a directive like 'CHECK: 26 00 65 7c sc $5, 0($3)' makes sense to me. If you update this review once you've made the change then I'll take a look at the new test.

It might be worth seeing if the -asm-show-inst to llc makes it possible to avoid assembling and disassembling in a CodeGen test. This option causes llc to emit the internal representation in a comment like so:

  move $fp, $sp  # <MCInst #1480 OR
                 #  <MCOperand Reg:8>
                 #  <MCOperand Reg:20>
                 #  <MCOperand Reg:21>>

In the case of 'sc', we should see 'SC' or 'SC_R6' depending on which was selected.

> Having in mind that existing (non micromips) atomic tests do not check instruction opcodes is it OK to create new test in this case?


Generally speaking, we don't need to check opcodes in CodeGen tests. It should be sufficient to check the mnemonic. sc/scd/ll/lld are a bit trickier since we have multiple copies of these instructions and we need to make sure we selected the correct 'sc'. I'd prefer to use -asm-show-inst if possible but if we need to check opcodes to tell them apart then we'll have to do that.

> I will add checks for ll, lld and scd opcodes.


Thanks


http://reviews.llvm.org/D13593