This is an archive of the discontinued LLVM Phabricator instance.

Differential D74919

AMDGPU/GlobalISel: Manually RegBankSelect copies
ClosedPublic

Authored by arsenm on Feb 20 2020, 11:24 AM.

Details

Reviewers
qcolombet
nhaehnle
foad
Summary

This was failng on any pre-assigned copy to the VCC bank.




This is something of a workaround for the default implementation in

getInstrMappingImpl, and how it treats copy-like operations in

general.






Copy-like operations are considered to only have one result register

bank, rather than separate banks for each source like a normal

instruction. To avoid potentially mishandling reg_sequence with

impossible operand combinations, the generic implementation errors on

impossible costs. If the bank was already assigned, is treated it

as-if it were an unsatisfiable REG_SEQUENCE mapping. We really don't

get any value from any of what getInstrMappingImpl tries to do for

copies, so just directly emit the simple mapping we really want.
Diff Detail
Event Timeline
arsenm created this revision.Feb 20 2020, 11:24 AM
Herald added a project: Restricted Project.  ·  View Herald TranscriptFeb 20 2020, 11:24 AM
Herald added subscribers: kerbowa, hiraditya, t-tye and 7 others.  ·  View Herald Transcript
arsenm added a comment.Mar 10 2020, 5:54 PM
ping
qcolombet accepted this revision.Mar 10 2020, 9:13 PM
qcolombet added inline comments.
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp


2993
You should be able to assert on this, like you suspected in your comment.



We use RPO, so sources should have been assigned at this point.
This revision is now accepted and ready to land.Mar 10 2020, 9:13 PM
arsenm closed this revision.Mar 17 2020, 8:09 AM
a2202f6a3f1c596d805755c2221a5b5527a1416b
Revision Contents
PathSize
llvm/
lib/
Target/
AMDGPU/
22 lines
test/
CodeGen/
AMDGPU/
GlobalISel/
181 lines
Diff 245699
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
Show First 20 Lines • Show All 2,977 Lines • ▼ Show 20 Lines
// Operands that must be SGPRs must accept potentially divergent VGPRs as
// Operands that must be SGPRs must accept potentially divergent VGPRs as

// legal. These will be dealt with in applyMappingImpl.
// legal. These will be dealt with in applyMappingImpl.

//
//

const RegisterBankInfo::InstructionMapping &
const RegisterBankInfo::InstructionMapping &

AMDGPURegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
AMDGPURegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {

  const MachineFunction &MF = *MI.getParent()->getParent();
  const MachineFunction &MF = *MI.getParent()->getParent();

  const MachineRegisterInfo &MRI = MF.getRegInfo();
  const MachineRegisterInfo &MRI = MF.getRegInfo();




  if (MI.isCopy()) {

    // The default logic bothers to analyze impossible alternative mappings. We

    // want the most straightforward mapping, so just directly handle this.

    const RegisterBank *DstBank = getRegBank(MI.getOperand(0).getReg(), MRI,

                                             *TRI);

    const RegisterBank *SrcBank = getRegBank(MI.getOperand(1).getReg(), MRI,

                                             *TRI);

    if (!SrcBank) // FIXME: Why would we see this?

qcolombetUnsubmitted
Not Done
ReplyInline Actions
You should be able to assert on this, like you suspected in your comment.



We use RPO, so sources should have been assigned at this point.
qcolombet: You should be able to assert on this, like you suspected in your comment.

We use RPO, so…
      SrcBank = &AMDGPU::VGPRRegBank;

    if (!DstBank)

      DstBank = SrcBank;



    unsigned Size = getSizeInBits(MI.getOperand(0).getReg(), MRI, *TRI);

    if (cannotCopy(*DstBank, *SrcBank, Size))

      return getInvalidInstructionMapping();



    const ValueMapping &ValMap = getValueMapping(0, Size, *DstBank);

    return getInstructionMapping(

        1, /*Cost*/ 1,

        /*OperandsMapping*/ getOperandsMapping({&ValMap}), 1);

  }



  if (MI.isRegSequence()) {
  if (MI.isRegSequence()) {

    // If any input is a VGPR, the result must be a VGPR. The default handling
    // If any input is a VGPR, the result must be a VGPR. The default handling

    // assumes any copy between banks is legal.
    // assumes any copy between banks is legal.

    unsigned BankID = AMDGPU::SGPRRegBankID;
    unsigned BankID = AMDGPU::SGPRRegBankID;




    for (unsigned I = 1, E = MI.getNumOperands(); I != E; I += 2) {
    for (unsigned I = 1, E = MI.getNumOperands(); I != E; I += 2) {

      auto OpBank = getRegBankID(MI.getOperand(I).getReg(), MRI, *TRI);
      auto OpBank = getRegBankID(MI.getOperand(I).getReg(), MRI, *TRI);

      // It doesn't make sense to use vcc or scc banks here, so just ignore
      // It doesn't make sense to use vcc or scc banks here, so just ignore

▲ Show 20 Lines • Show All 995 Lines • Show Last 20 Lines
llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-copy.mir
  • This file was added.
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py

# RUN: llc -march=amdgcn -mcpu=gfx908 -run-pass=regbankselect -regbankselect-fast -verify-machineinstrs %s -o - | FileCheck %s

# RUN: llc -march=amdgcn -mcpu=gfx908 -run-pass=regbankselect -regbankselect-greedy -verify-machineinstrs %s -o - | FileCheck %s



---

name: copy_s32_vgpr_to_vgpr

legalized: true



body: |

  bb.0:

    liveins: $vgpr0

    ; CHECK-LABEL: name: copy_s32_vgpr_to_vgpr

    ; CHECK: [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0

    ; CHECK: $vgpr0 = COPY [[COPY]](s32)

    %0:_(s32) = COPY $vgpr0

    $vgpr0 = COPY %0



...



---

name: copy_s32_sgpr_to_sgpr

legalized: true



body: |

  bb.0:

    liveins: $sgpr0

    ; CHECK-LABEL: name: copy_s32_sgpr_to_sgpr

    ; CHECK: [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0

    ; CHECK: $sgpr0 = COPY [[COPY]](s32)

    %0:_(s32) = COPY $sgpr0

    $sgpr0 = COPY %0



...



---

name: copy_s32_sgpr_to_vgpr

legalized: true



body: |

  bb.0:

    liveins: $sgpr0

    ; CHECK-LABEL: name: copy_s32_sgpr_to_vgpr

    ; CHECK: [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0

    ; CHECK: $vgpr0 = COPY [[COPY]](s32)

    %0:_(s32) = COPY $sgpr0

    $vgpr0 = COPY %0



...



---

name: copy_s32_vgpr_to_agpr

legalized: true



body: |

  bb.0:

    liveins: $vgpr0

    ; CHECK-LABEL: name: copy_s32_vgpr_to_agpr

    ; CHECK: [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0

    ; CHECK: $agpr0 = COPY [[COPY]](s32)

    %0:_(s32) = COPY $vgpr0

    $agpr0 = COPY %0



...



---

name: copy_s32_sgpr_to_agpr

legalized: true



body: |

  bb.0:

    liveins: $sgpr0

    ; CHECK-LABEL: name: copy_s32_sgpr_to_agpr

    ; CHECK: [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0

    ; CHECK: $agpr0 = COPY [[COPY]](s32)

    %0:_(s32) = COPY $sgpr0

    $agpr0 = COPY %0



...



---

name: copy_s32_agpr_to_vgpr

legalized: true



body: |

  bb.0:

    liveins: $agpr0

    ; CHECK-LABEL: name: copy_s32_agpr_to_vgpr

    ; CHECK: [[COPY:%[0-9]+]]:agpr(s32) = COPY $agpr0

    ; CHECK: $vgpr0 = COPY [[COPY]](s32)

    %0:_(s32) = COPY $agpr0

    $vgpr0 = COPY %0



...



---

name: copy_s32_agpr_to_agpr

legalized: true



body: |

  bb.0:

    liveins: $agpr0

    ; CHECK-LABEL: name: copy_s32_agpr_to_agpr

    ; CHECK: [[COPY:%[0-9]+]]:agpr(s32) = COPY $agpr0

    ; CHECK: $agpr0 = COPY [[COPY]](s32)

    %0:_(s32) = COPY $agpr0

    $agpr0 = COPY %0



...



---

name: copy_s1_sgpr_to_vcc_preassigned

legalized: true



body: |

  bb.0:

    liveins: $sgpr0

    ; CHECK-LABEL: name: copy_s1_sgpr_to_vcc_preassigned

    ; CHECK: [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0

    ; CHECK: [[TRUNC:%[0-9]+]]:sgpr(s1) = G_TRUNC [[COPY]](s32)

    ; CHECK: [[COPY1:%[0-9]+]]:vcc(s1) = COPY [[TRUNC]](s1)

    ; CHECK: S_ENDPGM 0, implicit [[COPY1]](s1)

    %0:sgpr(s32) = COPY $sgpr0

    %1:sgpr(s1) = G_TRUNC %0

    %2:vcc(s1) = COPY %1

    S_ENDPGM 0, implicit %2

...



---

name: copy_s1_vgpr_to_vcc_preassigned

legalized: true



body: |

  bb.0:

    liveins: $vgpr0

    ; CHECK-LABEL: name: copy_s1_vgpr_to_vcc_preassigned

    ; CHECK: [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0

    ; CHECK: [[TRUNC:%[0-9]+]]:vgpr(s1) = G_TRUNC [[COPY]](s32)

    ; CHECK: [[COPY1:%[0-9]+]]:vcc(s1) = COPY [[TRUNC]](s1)

    ; CHECK: S_ENDPGM 0, implicit [[COPY1]](s1)

    %0:vgpr(s32) = COPY $vgpr0

    %1:vgpr(s1) = G_TRUNC %0

    %2:vcc(s1) = COPY %1

    S_ENDPGM 0, implicit %2

...



---

name: copy_s1_sgpr_to_vcc

legalized: true



body: |

  bb.0:

    liveins: $sgpr0

    ; CHECK-LABEL: name: copy_s1_sgpr_to_vcc

    ; CHECK: [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0

    ; CHECK: [[TRUNC:%[0-9]+]]:sgpr(s1) = G_TRUNC [[COPY]](s32)

    ; CHECK: [[COPY1:%[0-9]+]]:vcc(s1) = COPY [[TRUNC]](s1)

    ; CHECK: S_ENDPGM 0, implicit [[COPY1]](s1)

    %0:_(s32) = COPY $sgpr0

    %1:_(s1) = G_TRUNC %0

    %2:vcc(s1) = COPY %1

    S_ENDPGM 0, implicit %2

...





---

name: copy_s1_vgpr_to_vcc

legalized: true



body: |

  bb.0:

    liveins: $vgpr0

    ; CHECK-LABEL: name: copy_s1_vgpr_to_vcc

    ; CHECK: [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0

    ; CHECK: [[TRUNC:%[0-9]+]]:vgpr(s1) = G_TRUNC [[COPY]](s32)

    ; CHECK: [[COPY1:%[0-9]+]]:vcc(s1) = COPY [[TRUNC]](s1)

    ; CHECK: S_ENDPGM 0, implicit [[COPY1]](s1)

    %0:_(s32) = COPY $vgpr0

    %1:_(s1) = G_TRUNC %0

    %2:vcc(s1) = COPY %1

    S_ENDPGM 0, implicit %2

...