Details
Diff Detail
Event Timeline
There is a file test/CodeGen/Mips/micromips-or16.ll which can be used for this test rather than adding a new file.
| lib/Target/Mips/MicroMips32r6InstrInfo.td | ||
|---|---|---|
| 814–815 | Can you put it on POOL16C_OR16_XOR16_FM_MMR6 instead? (The other examples of MicroMipsR6Inst16 in this file are in the wrong place too) | |
| test/CodeGen/Mips/micromips-or16.ll | ||
| 3–43 ↗ | (On Diff #48326) | This should be sufficient: define i32 @f(i32 signext %a, i32 signext %b) {
%1 = or i32 %a, %b
ret i32 %1
}
; CHECK-LABEL: f:
; CHECK-NOT: OR16_MMR6 |
I put MicroMipsR6Inst16 on POOL16C_OR16_XOR16_FM_MMR6, as you suggested.
The test-case you suggested would not work since it would always generate right OR16_MM instruction. However, I managed to simplify a bit the test case used last time.
The test case you suggested does not trigger this bug due to the way FastISel functions. It always first calls method FastISel::selectOperator which can do fast instruction selection for a small set of operators. Then, if selectOperator fails, FastISel calls MipsFastISel::fastSelectInstruction which checks if the architecture for which the code is generated is supported. If FastISel fails for one instruction, it does not continue trying for other instructions in that basic block. It selects instructions in reverse order.
In the test-case you suggested, FastISel tries to select an instruction for RET by selectOperator method and it fails. Then it tries to select an instruction by fastSelectInstruction, which again fails since microMips is not supported architecture, and then FastISel stops and instruction selection is continued in a normal way (and there is no bug).
In the test-case suggested in this patch, all instructions before OR are selected by selectOperator method, and then this method selects (wrong) OR16_MMR6 instruction.
In that case we just need to split the bb in the small test case. The code below emits an OR16_MMR6 on my build.
| test/CodeGen/Mips/micromips-or16.ll | ||
|---|---|---|
| 3–43 ↗ | (On Diff #49369) | This is the simplest test I can find that emits an OR16_MMR6: define i32 @f(i32 signext %a, i32 signext %b) {
%1 = or i32 %a, %b
br label %b1
b1:
ret i32 %1
} |
Can you put it on POOL16C_OR16_XOR16_FM_MMR6 instead?
(The other examples of MicroMipsR6Inst16 in this file are in the wrong place too)