Page MenuHomePhabricator

[RISCV][GlobalISel] Select ALU GPR instructions
Needs ReviewPublic

Authored by lewis-revill on Mar 19 2020, 12:22 PM.

Details

Summary

Some instruction selection patterns required for ALU GPR instructions have already been automatically imported from existing TableGen descriptions - this patch simply adds testing for them. Logic for selecting constants and copies has been added, along with the first of the GIComplexPatternEquiv definitions required to select the shiftMaskXLen ComplexPattern. New patterns have been added to directly select RV64 W instructions from gMIR rather than using custom gMIR operations earlier in the pipeline. In future this could also support the checks present in the DAGToDAGISel for finding ops whose users only require the lower 32 bits.

Diff Detail

Event Timeline

lewis-revill created this revision.Mar 19 2020, 12:22 PM

Add tests for AND/OR/XOR.

Use utils/update_mir_test_checks.py

lewis-revill retitled this revision from [RISCV][GlobalISel] Add tests for selecting ALU GPR instructions to [WIP][RISCV][GlobalISel] Select ALU GPR instructions.
lewis-revill edited the summary of this revision. (Show Details)

Add custom selection for copies and for constants. Significantly expand tests over more types.

Bug fix - ensure selectConstant produces copies with fully constrained registers.

arsenm added a subscriber: arsenm.Aug 11 2020, 12:36 PM

Missing copy tests for all permutations of virtual and physical registers

llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp
114–115

This can't early return, it still needs to constrain a virtual source

lewis-revill edited the summary of this revision. (Show Details)

Rebased. Added the first GIComplexPatternEquiv definition in order to handle the operands to shifts, which are now covered by a ComplexPattern in SelectionDAG. Updated the tests to match the output of the regbank selector.

lewis-revill planned changes to this revision.Jan 14 2022, 7:52 AM

Still need to address testing permutations of copying virtual/physical registers.

lenary removed a subscriber: lenary.Jan 14 2022, 7:53 AM
rkruppe removed a subscriber: rkruppe.Jan 14 2022, 8:56 AM
arsenm added inline comments.Jan 17 2022, 4:02 PM
llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp
95–106

This function name is misleading. Can you restructure to use something more like getRegClassForTypeOnBank (which AArch64 has a FIXME to make a generic TargetRegisterInfo method)

lewis-revill edited the summary of this revision. (Show Details)

Restructure slightly, add more cases for selectCopy along with tests, add selection patterns for RV64 W instructions.

lewis-revill marked 2 inline comments as done.Feb 1 2022, 7:23 AM
lewis-revill retitled this revision from [WIP][RISCV][GlobalISel] Select ALU GPR instructions to [RISCV][GlobalISel] Select ALU GPR instructions.Feb 9 2022, 3:14 AM
arsenm added inline comments.Feb 9 2022, 8:31 AM
llvm/lib/Target/RISCV/RISCVGISel.td
30 ↗(On Diff #404931)

Should sext_inreg be legal?

llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp
45

What is this parameter?

107

SrcReg.isPhysical()

118

Why not just print the whole instruction?

143

Should not construct one off MachineIRBuilders. The selector usually just directly uses BuildMI

169

Didn't erase the instruction

lewis-revill added inline comments.Apr 7 2022, 5:31 AM
llvm/lib/Target/RISCV/RISCVGISel.td
30 ↗(On Diff #404931)

This is what I'm stuck on right now. It really should to match the SelectionDAG code, at least for i32 -> i64. However there is no way to map G_SEXT_INREG to sext_inreg because sext_inreg uses a value type as a DAG operand, whereas G_SEXT_INREG uses a constant which indicates the size of the original type. So no patterns defined which use sext_inreg will work for GlobalISel. I think my only option left is to do custom selection code.

Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2022, 5:31 AM
arsenm added inline comments.Apr 7 2022, 6:21 AM
llvm/lib/Target/RISCV/RISCVGISel.td
30 ↗(On Diff #404931)

You can either do custom selection code, or you could add support for sext_inreg in GlobalISelEmitter. It's a special case so it requires special support