This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][GlobalISel] Select G_ICMP
AbandonedPublic

Authored by craig.topper on Aug 29 2023, 11:52 AM.

Details

Summary

Add TableGen patterns to select the integer predicates for G_ICMP. We do not select for comparisons between pointers yet.

Diff Detail

Event Timeline

nitinjohnraj created this revision.Aug 29 2023, 11:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2023, 11:52 AM
nitinjohnraj requested review of this revision.Aug 29 2023, 11:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2023, 11:52 AM
craig.topper added inline comments.Aug 29 2023, 4:03 PM
llvm/lib/Target/RISCV/RISCVGISel.td
75

This pattern isn't working

80

This pattern isn't working

94

This pattern isn't working

nitinjohnraj added inline comments.Aug 30 2023, 3:48 PM
llvm/lib/Target/RISCV/RISCVGISel.td
19

Can't use SDNodeXForm, look into replacing this with a ComplexPattern

Remove patterns that aren't working. Will fix them.

nitinjohnraj marked 3 inline comments as done.Aug 30 2023, 4:37 PM

Make it work for the eq0, ne0, sgti cases

nitinjohnraj added inline comments.Sep 5 2023, 1:42 PM
llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/cmp64.mir
4

Add pointer tests

nitinjohnraj edited the summary of this revision. (Show Details)Sep 5 2023, 2:26 PM
nitinjohnraj added inline comments.
llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/cmp64.mir
4

Requires G_PTRTOINT, will be done in future patch

craig.topper added inline comments.Sep 5 2023, 3:03 PM
llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
176

We don't have to handle this case explicitly as long as we don't match ugt X, -1 in the tablegen pattern. It's completely fine to emit

li a1, -1
sltu a0, a1, a0

Remove special handling for (setugt rs1, -1) case. Add support for comparing pointers.

craig.topper added inline comments.Sep 6 2023, 4:22 PM
llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
205

Indent

llvm/lib/Target/RISCV/RISCVGISel.td
71

Is this FIXME still needed?

96

simm12Minus1Nonzero -> simm12Minus1NonzeroNonNeg1

98

Need

def : Pat<(XLenVT (setge (XLenVT GPR:$rs1), simm12:$imm)),
          (XORI (SLTI GPR:$rs1, simm12:$imm), 1)>;
def : Pat<(XLenVT (setuge (XLenVT GPR:$rs1), simm12:$imm)),
          (XORI (SLTIU GPR:$rs1, simm12:$imm), 1)>;

def : Pat<(XLenVT (setle (XLenVT GPR:$rs1), simm12Minus1Nonzero:$imm)),
         (SLTI GPR:$rs1, (ImmPlus1 simm12Minus1Nonzero:$imm))>;
def : Pat<(XLenVT (setule (XLenVT GPR:$rs1), simm12Minus1NonzeroNonNeg1:$imm)),
          (SLTIU GPR:$rs1,
                       (ImmPlus1 simm12Minus1NonzeroNonNeg1:$imm))>;
craig.topper added inline comments.Sep 6 2023, 5:11 PM
llvm/lib/Target/RISCV/RISCVGISel.td
24

Imm >= -2049

27

Imm >= -2049

llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/cmp32.mir
798

While this is the code we want for this case, its only work because no negative immediates are being folded for some reason.

nitinjohnraj marked 3 inline comments as done.

Correct bound errors, edit tests to check for negative numbers and add relevant rules to tablegen.

nitinjohnraj marked 5 inline comments as done.Sep 8 2023, 10:48 AM

Need tests for comparing pointers to G_CONSTANT

llvm/lib/Target/RISCV/RISCVGISel.td
98

Still missing setle and setule patterns with immediates

arsenm added inline comments.Sep 12 2023, 8:34 AM
llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
167

This is pretty ugly, I thought tablegen already did something for the pointer types to the int patterns here?

Added missing rules for sle and ule with immediates. Cleaned up the lowering code for pointers.

This patch needs to be rebased on the current version of the ALU patch

craig.topper added inline comments.Sep 12 2023, 2:31 PM
llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
167

We found this patch that was never merged https://reviews.llvm.org/D55914 . Should we re-consider that patch?

craig.topper commandeered this revision.Wed, Nov 15, 2:30 PM
craig.topper abandoned this revision.
craig.topper edited reviewers, added: nitinjohnraj; removed: craig.topper.