Commit 3d02fa6d authored by Volkan Keles's avatar Volkan Keles
Browse files

[GlobalISel] CombinerHelper: Fix a bug in matchCombineCopy 

Summary:
When combining COPY instructions, we were replacing the destination registers
with the source register without checking register constraints. This patch adds
a simple logic to check if the constraints match before replacing registers.

Reviewers: qcolombet, aditya_nandakumar, aemerson, paquette, dsanders, Petar.Avramovic

Reviewed By: aditya_nandakumar

Subscribers: rovka, hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D70616
parent 469ee617

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

+26 −3
Original line number Diff line number Diff line
@@ -74,12 +74,35 @@ bool CombinerHelper::matchCombineCopy(MachineInstr &MI) { 
    return false;

  Register DstReg = MI.getOperand(0).getReg();

  Register SrcReg = MI.getOperand(1).getReg();



  // Give up if either DstReg or SrcReg  is a physical register.

  if (Register::isPhysicalRegister(DstReg) ||

      Register::isPhysicalRegister(SrcReg))

    return false;



  // Give up the types don't match.

  LLT DstTy = MRI.getType(DstReg);

  LLT SrcTy = MRI.getType(SrcReg);

  // Simple Copy Propagation.

  // a(sx) = COPY b(sx) -> Replace all uses of a with b.

  if (DstTy.isValid() && SrcTy.isValid() && DstTy == SrcTy)

  // Give up if one has a valid LLT, but the other doesn't.

  if (DstTy.isValid() != SrcTy.isValid())

    return false;

  // Give up if the types don't match.

  if (DstTy.isValid() && SrcTy.isValid() && DstTy != SrcTy)

    return false;



  // Get the register banks and classes.

  const RegisterBank *DstBank = MRI.getRegBankOrNull(DstReg);

  const RegisterBank *SrcBank = MRI.getRegBankOrNull(SrcReg);

  const TargetRegisterClass *DstRC = MRI.getRegClassOrNull(DstReg);

  const TargetRegisterClass *SrcRC = MRI.getRegClassOrNull(SrcReg);



  // Replace if the register constraints match.

  if ((SrcRC == DstRC) && (SrcBank == DstBank))

    return true;

  // Replace if DstReg has no constraints.

  if (!DstBank && !DstRC)

    return true;



  return false;

}

void CombinerHelper::applyCombineCopy(MachineInstr &MI) {

llvm/test/CodeGen/AArch64/GlobalISel/combine-copy.mir

0 → 100644
+86 −0
Original line number Diff line number Diff line 
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py

# RUN: llc -o - -march=aarch64 -run-pass=aarch64-prelegalizer-combiner %s | FileCheck %s



# Make sure we don't lose the register bank constraints when

# combining COPY instructions.

---

name:            test_none_none

body:             |

  bb.0.entry:

    ; CHECK-LABEL: name: test_none_none

    ; CHECK: [[COPY:%[0-9]+]]:_(s64) = COPY $x0

    ; CHECK: $x0 = COPY [[COPY]](s64)

    %0:_(s64) = COPY $x0

    %1:_(s64) = COPY %0(s64)

    $x0 = COPY %1(s64)

...

---

name:            test_gpr_none

body:             |

  bb.0.entry:

    ; CHECK-LABEL: name: test_gpr_none

    ; CHECK: [[COPY:%[0-9]+]]:gpr(s64) = COPY $x0

    ; CHECK: $x0 = COPY [[COPY]](s64)

    %0:gpr(s64) = COPY $x0

    %1:_(s64) = COPY %0(s64)

    $x0 = COPY %1(s64)

...

---

name:            test_none_gpr

body:             |

  bb.0.entry:

    ; CHECK-LABEL: name: test_none_gpr

    ; CHECK: [[COPY:%[0-9]+]]:_(s64) = COPY $x0

    ; CHECK: [[COPY1:%[0-9]+]]:gpr(s64) = COPY [[COPY]](s64)

    ; CHECK: $x0 = COPY [[COPY1]](s64)

    %0:_(s64) = COPY $x0

    %1:gpr(s64) = COPY %0(s64)

    $x0 = COPY %1(s64)

...

---

name:            test_fpr_gpr

body:             |

  bb.0.entry:

    ; CHECK-LABEL: name: test_fpr_gpr

    ; CHECK: [[COPY:%[0-9]+]]:fpr(s64) = COPY $x0

    ; CHECK: [[COPY1:%[0-9]+]]:gpr(s64) = COPY [[COPY]](s64)

    ; CHECK: $x0 = COPY [[COPY1]](s64)

    %0:fpr(s64) = COPY $x0

    %1:gpr(s64) = COPY %0(s64)

    $x0 = COPY %1(s64)

...

---

name:            test_gpr64_gpr64_dst_no_llt

body:             |

  bb.0.entry:

    ; CHECK-LABEL: name: test_gpr64_gpr64_dst_no_llt

    ; CHECK: [[COPY:%[0-9]+]]:gpr64(s64) = COPY $x0

    ; CHECK: [[COPY1:%[0-9]+]]:gpr64 = COPY [[COPY]](s64)

    ; CHECK: $x0 = COPY [[COPY1]]

    %0:gpr64(s64) = COPY $x0

    %1:gpr64 = COPY %0(s64)

    $x0 = COPY %1

...

---

name:            test_gpr64_gpr64_src_no_llt

body:             |

  bb.0.entry:

    ; CHECK-LABEL: name: test_gpr64_gpr64_src_no_llt

    ; CHECK: [[COPY:%[0-9]+]]:gpr64 = COPY $x0

    ; CHECK: [[COPY1:%[0-9]+]]:gpr64(s64) = COPY [[COPY]]

    ; CHECK: $x0 = COPY [[COPY1]](s64)

    %0:gpr64 = COPY $x0

    %1:gpr64(s64) = COPY %0

    $x0 = COPY %1(s64)

...

---

name:            test_gpr64_gpr64_both_no_llt

body:             |

  bb.0.entry:

    ; CHECK-LABEL: name: test_gpr64_gpr64_both_no_llt

    ; CHECK: [[COPY:%[0-9]+]]:gpr64 = COPY $x0

    ; CHECK: $x0 = COPY [[COPY]]

    %0:gpr64 = COPY $x0

    %1:gpr64 = COPY %0

    $x0 = COPY %1

...