This is an archive of the discontinued LLVM Phabricator instance.

[mips] Map SW instruction to its microMIPS R6 variant
ClosedPublic

Authored by atanasyan on Mar 6 2019, 1:35 PM.

Details

Summary

To provide mapping between standard and microMIPS R6 variants of the sw command we have to rename SWSP_xxx commands from "sw" to "swsp". Otherwise tablegen starts to show the error Multiple matches found for SW'. After that to restore printing SWSP command as sw`, I add an appropriate MipsInstAlias instance.

We also need to implement "size reduction" for microMIPS R6. But this task is for separate patch. After that the micromips-lwsp-swsp.ll test case will be extended.

Diff Detail

Repository
rL LLVM

Event Timeline

atanasyan created this revision.Mar 6 2019, 1:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 6 2019, 1:35 PM

SWSP instruction has the same encoding for both standard and r6 microMIPS (LOAD_STORE_SP_FM_MM16<0x32>).
If we change SWSP_MM from ISA_MICROMIPS32_NOT_MIPS32R6 to ISA_MICROMIPS, SWSP_MM could be used for R6 size reduction also.
SWSP_MMR6 looks redundant?

SWSP instruction has the same encoding for both standard and r6 microMIPS (LOAD_STORE_SP_FM_MM16<0x32>).
If we change SWSP_MM from ISA_MICROMIPS32_NOT_MIPS32R6 to ISA_MICROMIPS, SWSP_MM could be used for R6 size reduction also.
SWSP_MMR6 looks redundant?

There some other places in the code where we emit Mips::SW instruction and rely on mapping provided by td files. For example, expanding ld and sd macros in the MipsAsmParser::expandLoadStoreDMacro; expanding .cprestore in the MipsTargetELFStreamer::emitDirectiveCpRestore. Now LLVM generates incorrect code in these cases. As in the D59045, we can either use td mapping or addd if (MicroMIPS) statements. What approach is better? As to me, I prefer *td* because in that case we keep changes in one place.

I see. I think that in order to keep up with current implementation of microMIPS r6 in MicroMips32r6InstrInfo.td we should map microMIPS instruction to its corresponding standard encoding instruction when necessary.
According to documentation cb e5 has assembler format swsp $ra, 20($sp) but since here both clang and gcc use assembler format sw $ra, 20($sp) I think that we should keep the same behavior.
I would prefer solution in which when target instruction is selected it cannot be changed later. Alternatively for efficiency pseudo instruction could be selected and expanded into target instruction later. This approach would allow faster localization if problem occurs.
Regarding microMIPS r6 implementation I think that we should add both " patterns / manual instruction select" and "instruction mapping" when possible for completeness, since both are present. "Instruction mapping" approach also introduces some code duplication since often there is no difference between microMIPS pre-r6 and r6 instructions.
I agree that mapping in .td file is the best solution here.

This revision is now accepted and ready to land.Mar 12 2019, 8:31 AM
This revision was automatically updated to reflect the committed changes.

Thanks for the thorough review.