[llvm] r267180 - [AArch64][AdvSIMDScalar] Update the kill flags correctly.
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 22 11:09:15 PDT 2016
Author: qcolombet
Date: Fri Apr 22 13:09:14 2016
New Revision: 267180
URL: http://llvm.org/viewvc/llvm-project?rev=267180&view=rev
Log:
[AArch64][AdvSIMDScalar] Update the kill flags correctly.
We used to simply set the kill flags to true when transforming a scalar
instruction to a vector one.
SrcScalar1 = copy SrcVector1
... = opScalar SrcScalar1
=>
SrcScalar1 = copy SrcVector1
... = opVector SrcVector1<kill>
This is obviously wrong. The proper update consists in:
1. Propagate the kill status from the copy to the new opVector
2. Reset the kill status on the copy, since the live-range of
SrcVector1 got extended.
This fixes some of the machine verifier errors for AArch64 with make check.
Modified:
llvm/trunk/lib/Target/AArch64/AArch64AdvSIMDScalarPass.cpp
llvm/trunk/test/CodeGen/AArch64/arm64-AdvSIMD-Scalar.ll
Modified: llvm/trunk/lib/Target/AArch64/AArch64AdvSIMDScalarPass.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64AdvSIMDScalarPass.cpp?rev=267180&r1=267179&r2=267180&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64AdvSIMDScalarPass.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64AdvSIMDScalarPass.cpp Fri Apr 22 13:09:14 2016
@@ -132,19 +132,19 @@ static bool isFPR64(unsigned Reg, unsign
// getSrcFromCopy - Get the original source register for a GPR64 <--> FPR64
// copy instruction. Return zero_reg if the instruction is not a copy.
-static unsigned getSrcFromCopy(const MachineInstr *MI,
- const MachineRegisterInfo *MRI,
- unsigned &SubReg) {
+static MachineOperand *getSrcFromCopy(MachineInstr *MI,
+ const MachineRegisterInfo *MRI,
+ unsigned &SubReg) {
SubReg = 0;
// The "FMOV Xd, Dn" instruction is the typical form.
if (MI->getOpcode() == AArch64::FMOVDXr ||
MI->getOpcode() == AArch64::FMOVXDr)
- return MI->getOperand(1).getReg();
+ return &MI->getOperand(1);
// A lane zero extract "UMOV.d Xd, Vn[0]" is equivalent. We shouldn't see
// these at this stage, but it's easy to check for.
if (MI->getOpcode() == AArch64::UMOVvi64 && MI->getOperand(2).getImm() == 0) {
SubReg = AArch64::dsub;
- return MI->getOperand(1).getReg();
+ return &MI->getOperand(1);
}
// Or just a plain COPY instruction. This can be directly to/from FPR64,
// or it can be a dsub subreg reference to an FPR128.
@@ -152,18 +152,18 @@ static unsigned getSrcFromCopy(const Mac
if (isFPR64(MI->getOperand(0).getReg(), MI->getOperand(0).getSubReg(),
MRI) &&
isGPR64(MI->getOperand(1).getReg(), MI->getOperand(1).getSubReg(), MRI))
- return MI->getOperand(1).getReg();
+ return &MI->getOperand(1);
if (isGPR64(MI->getOperand(0).getReg(), MI->getOperand(0).getSubReg(),
MRI) &&
isFPR64(MI->getOperand(1).getReg(), MI->getOperand(1).getSubReg(),
MRI)) {
SubReg = MI->getOperand(1).getSubReg();
- return MI->getOperand(1).getReg();
+ return &MI->getOperand(1);
}
}
// Otherwise, this is some other kind of instruction.
- return 0;
+ return nullptr;
}
// getTransformOpcode - For any opcode for which there is an AdvSIMD equivalent
@@ -211,31 +211,31 @@ AArch64AdvSIMDScalar::isProfitableToTran
unsigned OrigSrc0 = MI->getOperand(1).getReg();
unsigned OrigSrc1 = MI->getOperand(2).getReg();
- unsigned Src0 = 0, SubReg0;
- unsigned Src1 = 0, SubReg1;
+ unsigned SubReg0;
+ unsigned SubReg1;
if (!MRI->def_empty(OrigSrc0)) {
MachineRegisterInfo::def_instr_iterator Def =
MRI->def_instr_begin(OrigSrc0);
assert(std::next(Def) == MRI->def_instr_end() && "Multiple def in SSA!");
- Src0 = getSrcFromCopy(&*Def, MRI, SubReg0);
+ MachineOperand *MOSrc0 = getSrcFromCopy(&*Def, MRI, SubReg0);
// If the source was from a copy, we don't need to insert a new copy.
- if (Src0)
+ if (MOSrc0)
--NumNewCopies;
// If there are no other users of the original source, we can delete
// that instruction.
- if (Src0 && MRI->hasOneNonDBGUse(OrigSrc0))
+ if (MOSrc0 && MRI->hasOneNonDBGUse(OrigSrc0))
++NumRemovableCopies;
}
if (!MRI->def_empty(OrigSrc1)) {
MachineRegisterInfo::def_instr_iterator Def =
MRI->def_instr_begin(OrigSrc1);
assert(std::next(Def) == MRI->def_instr_end() && "Multiple def in SSA!");
- Src1 = getSrcFromCopy(&*Def, MRI, SubReg1);
- if (Src1)
+ MachineOperand *MOSrc1 = getSrcFromCopy(&*Def, MRI, SubReg1);
+ if (MOSrc1)
--NumNewCopies;
// If there are no other users of the original source, we can delete
// that instruction.
- if (Src1 && MRI->hasOneNonDBGUse(OrigSrc1))
+ if (MOSrc1 && MRI->hasOneNonDBGUse(OrigSrc1))
++NumRemovableCopies;
}
@@ -306,30 +306,43 @@ void AArch64AdvSIMDScalar::transformInst
unsigned OrigSrc1 = MI->getOperand(2).getReg();
unsigned Src0 = 0, SubReg0;
unsigned Src1 = 0, SubReg1;
+ bool KillSrc0 = false, KillSrc1 = false;
if (!MRI->def_empty(OrigSrc0)) {
MachineRegisterInfo::def_instr_iterator Def =
MRI->def_instr_begin(OrigSrc0);
assert(std::next(Def) == MRI->def_instr_end() && "Multiple def in SSA!");
- Src0 = getSrcFromCopy(&*Def, MRI, SubReg0);
+ MachineOperand *MOSrc0 = getSrcFromCopy(&*Def, MRI, SubReg0);
// If there are no other users of the original source, we can delete
// that instruction.
- if (Src0 && MRI->hasOneNonDBGUse(OrigSrc0)) {
- assert(Src0 && "Can't delete copy w/o a valid original source!");
- Def->eraseFromParent();
- ++NumCopiesDeleted;
+ if (MOSrc0) {
+ Src0 = MOSrc0->getReg();
+ KillSrc0 = MOSrc0->isKill();
+ // Src0 is going to be reused, thus, it cannot be killed anymore.
+ MOSrc0->setIsKill(false);
+ if (MRI->hasOneNonDBGUse(OrigSrc0)) {
+ assert(MOSrc0 && "Can't delete copy w/o a valid original source!");
+ Def->eraseFromParent();
+ ++NumCopiesDeleted;
+ }
}
}
if (!MRI->def_empty(OrigSrc1)) {
MachineRegisterInfo::def_instr_iterator Def =
MRI->def_instr_begin(OrigSrc1);
assert(std::next(Def) == MRI->def_instr_end() && "Multiple def in SSA!");
- Src1 = getSrcFromCopy(&*Def, MRI, SubReg1);
+ MachineOperand *MOSrc1 = getSrcFromCopy(&*Def, MRI, SubReg1);
// If there are no other users of the original source, we can delete
// that instruction.
- if (Src1 && MRI->hasOneNonDBGUse(OrigSrc1)) {
- assert(Src1 && "Can't delete copy w/o a valid original source!");
- Def->eraseFromParent();
- ++NumCopiesDeleted;
+ if (MOSrc1) {
+ Src1 = MOSrc1->getReg();
+ KillSrc1 = MOSrc1->isKill();
+ // Src0 is going to be reused, thus, it cannot be killed anymore.
+ MOSrc1->setIsKill(false);
+ if (MRI->hasOneNonDBGUse(OrigSrc1)) {
+ assert(MOSrc1 && "Can't delete copy w/o a valid original source!");
+ Def->eraseFromParent();
+ ++NumCopiesDeleted;
+ }
}
}
// If we weren't able to reference the original source directly, create a
@@ -337,12 +350,14 @@ void AArch64AdvSIMDScalar::transformInst
if (!Src0) {
SubReg0 = 0;
Src0 = MRI->createVirtualRegister(&AArch64::FPR64RegClass);
- insertCopy(TII, MI, Src0, OrigSrc0, true);
+ insertCopy(TII, MI, Src0, OrigSrc0, KillSrc0);
+ KillSrc0 = true;
}
if (!Src1) {
SubReg1 = 0;
Src1 = MRI->createVirtualRegister(&AArch64::FPR64RegClass);
- insertCopy(TII, MI, Src1, OrigSrc1, true);
+ insertCopy(TII, MI, Src1, OrigSrc1, KillSrc1);
+ KillSrc1 = true;
}
// Create a vreg for the destination.
@@ -354,8 +369,8 @@ void AArch64AdvSIMDScalar::transformInst
// form, so no need to special case based on what instruction we're
// building.
BuildMI(*MBB, MI, MI->getDebugLoc(), TII->get(NewOpc), Dst)
- .addReg(Src0, getKillRegState(true), SubReg0)
- .addReg(Src1, getKillRegState(true), SubReg1);
+ .addReg(Src0, getKillRegState(KillSrc0), SubReg0)
+ .addReg(Src1, getKillRegState(KillSrc1), SubReg1);
// Now copy the result back out to a GPR.
// FIXME: Try to avoid this if all uses could actually just use the FPR64
Modified: llvm/trunk/test/CodeGen/AArch64/arm64-AdvSIMD-Scalar.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/arm64-AdvSIMD-Scalar.ll?rev=267180&r1=267179&r2=267180&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/arm64-AdvSIMD-Scalar.ll (original)
+++ llvm/trunk/test/CodeGen/AArch64/arm64-AdvSIMD-Scalar.ll Fri Apr 22 13:09:14 2016
@@ -1,7 +1,7 @@
-; RUN: llc < %s -march=arm64 -aarch64-neon-syntax=apple -aarch64-simd-scalar=true -asm-verbose=false -disable-adv-copy-opt=true | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-NOOPT
-; RUN: llc < %s -march=arm64 -aarch64-neon-syntax=apple -aarch64-simd-scalar=true -asm-verbose=false -disable-adv-copy-opt=false | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-OPT
-; RUN: llc < %s -march=arm64 -aarch64-neon-syntax=generic -aarch64-simd-scalar=true -asm-verbose=false -disable-adv-copy-opt=true | FileCheck %s -check-prefix=GENERIC -check-prefix=GENERIC-NOOPT
-; RUN: llc < %s -march=arm64 -aarch64-neon-syntax=generic -aarch64-simd-scalar=true -asm-verbose=false -disable-adv-copy-opt=false | FileCheck %s -check-prefix=GENERIC -check-prefix=GENERIC-OPT
+; RUN: llc < %s -verify-machineinstrs -march=arm64 -aarch64-neon-syntax=apple -aarch64-simd-scalar=true -asm-verbose=false -disable-adv-copy-opt=true | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-NOOPT
+; RUN: llc < %s -verify-machineinstrs -march=arm64 -aarch64-neon-syntax=apple -aarch64-simd-scalar=true -asm-verbose=false -disable-adv-copy-opt=false | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-OPT
+; RUN: llc < %s -verify-machineinstrs -march=arm64 -aarch64-neon-syntax=generic -aarch64-simd-scalar=true -asm-verbose=false -disable-adv-copy-opt=true | FileCheck %s -check-prefix=GENERIC -check-prefix=GENERIC-NOOPT
+; RUN: llc < %s -verify-machineinstrs -march=arm64 -aarch64-neon-syntax=generic -aarch64-simd-scalar=true -asm-verbose=false -disable-adv-copy-opt=false | FileCheck %s -check-prefix=GENERIC -check-prefix=GENERIC-OPT
define <2 x i64> @bar(<2 x i64> %a, <2 x i64> %b) nounwind readnone {
; CHECK-LABEL: bar:
More information about the llvm-commits
mailing list