Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td | ||
---|---|---|
1958–1959 | Is there some reason to pass AArch64ld1ro as an argument, as opposed to hardcoding it in sve_mem_ldor_ss? | |
llvm/lib/Target/AArch64/SVEInstrFormats.td | ||
7709 | Is the AddedComplexity here actually necessary? | |
llvm/test/CodeGen/AArch64/sve-intrinsics-ld1ro-addressing-mode-reg-reg.ll | ||
81 | I'd like to see a couple testcases where the pattern doesn't match; wrong shift, or subtraction, or something like that |
Thank you for the review @efriedma.
Francesco
llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td | ||
---|---|---|
1958–1959 | I think it is just a convention. The defs of these custom SDNodes are in AArch64SVEInstrInfo.td, not in its include file SVEInstrFormats.td, which is where the multiclass is defined. For example, see: frapet01@man-08:~/projects/upstream-clang/llvm-project/llvm/lib/Target/AArch64 (master)$ grep AArch64ld1 *.td | head -n 30 AArch64SVEInstrInfo.td:def AArch64ld1 : SDNode<"AArch64ISD::LD1", SDT_AArch64_LD1, [SDNPHasChain, SDNPMayLoad, SDNPOptInGlue]>; AArch64SVEInstrInfo.td:def AArch64ld1s : SDNode<"AArch64ISD::LD1S", SDT_AArch64_LD1, [SDNPHasChain, SDNPMayLoad, SDNPOptInGlue]>; AArch64SVEInstrInfo.td:def AArch64ld1rq : SDNode<"AArch64ISD::LD1RQ", SDT_AArch64_LD1Replicate, [SDNPHasChain, SDNPMayLoad]>; AArch64SVEInstrInfo.td:def AArch64ld1ro : SDNode<"AArch64ISD::LD1RO", SDT_AArch64_LD1Replicate, [SDNPHasChain, SDNPMayLoad]>; [...] AArch64SVEInstrInfo.td: defm GLD1B_S : sve_mem_32b_gld_vi_32_ptrs<0b0010, "ld1b", imm0_31, AArch64ld1_gather_imm, nxv4i8>; AArch64SVEInstrInfo.td: defm GLD1SH_S : sve_mem_32b_gld_vi_32_ptrs<0b0100, "ld1sh", uimm5s2, AArch64ld1s_gather_imm, nxv4i16>; AArch64SVEInstrInfo.td: defm GLD1H_S : sve_mem_32b_gld_vi_32_ptrs<0b0110, "ld1h", uimm5s2, AArch64ld1_gather_imm, nxv4i16>; | |
llvm/lib/Target/AArch64/SVEInstrFormats.td | ||
7709 | Ah right, not needed in this patch. It will probably be needed when we introduce the reg+imm optimization. In any case, I have removed it from here. Thank you for the head out. | |
llvm/test/CodeGen/AArch64/sve-intrinsics-ld1ro-addressing-mode-reg-reg.ll | ||
81 | Isn't this covered by the tests in https://reviews.llvm.org/D80738? Those tests are saying: "default to [xBASE] if you cannot figure out an optimal addressing mode supported by the instruction". Or am I missing something? |
Is there some reason to pass AArch64ld1ro as an argument, as opposed to hardcoding it in sve_mem_ldor_ss?