Skip to content

Commit

Permalink
[InstrInfo] Use 64-bit immediates for analyzeCompare() (NFCI)
Browse files Browse the repository at this point in the history
The backend generally uses 64-bit immediates (e.g. what
MachineOperand::getImm() returns), so use that for analyzeCompare()
and optimizeCompareInst() as well. This avoids truncation for
targets that support immediates larger 32-bit. In particular, we
can avoid the bugprone value normalization hack in the AArch64
target.

This is a followup to D108076.

Differential Revision: https://reviews.llvm.org/D108875
  • Loading branch information
nikic committed Aug 30, 2021
1 parent ed4946f commit 0529e2e
Show file tree
Hide file tree
Showing 18 changed files with 61 additions and 66 deletions.
6 changes: 4 additions & 2 deletions llvm/include/llvm/CodeGen/TargetInstrInfo.h
Expand Up @@ -1537,15 +1537,17 @@ class TargetInstrInfo : public MCInstrInfo {
/// compares against in CmpValue. Return true if the comparison instruction
/// can be analyzed.
virtual bool analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &Mask, int &Value) const {
Register &SrcReg2, int64_t &Mask,
int64_t &Value) const {
return false;
}

/// See if the comparison instruction can be converted
/// into something more efficient. E.g., on ARM most instructions can set the
/// flags register, obviating the need for a separate CMP.
virtual bool optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
Register SrcReg2, int Mask, int Value,
Register SrcReg2, int64_t Mask,
int64_t Value,
const MachineRegisterInfo *MRI) const {
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/PeepholeOptimizer.cpp
Expand Up @@ -626,7 +626,7 @@ bool PeepholeOptimizer::optimizeCmpInstr(MachineInstr &MI) {
// If this instruction is a comparison against zero and isn't comparing a
// physical register, we can try to optimize it.
Register SrcReg, SrcReg2;
int CmpMask, CmpValue;
int64_t CmpMask, CmpValue;
if (!TII->analyzeCompare(MI, SrcReg, SrcReg2, CmpMask, CmpValue) ||
SrcReg.isPhysical() || SrcReg2.isPhysical())
return false;
Expand Down
27 changes: 7 additions & 20 deletions llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
Expand Up @@ -1112,24 +1112,14 @@ bool AArch64InstrInfo::isSchedulingBoundary(const MachineInstr &MI,
/// in SrcReg and SrcReg2, and the value it compares against in CmpValue.
/// Return true if the comparison instruction can be analyzed.
bool AArch64InstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &CmpMask,
int &CmpValue) const {
Register &SrcReg2, int64_t &CmpMask,
int64_t &CmpValue) const {
// The first operand can be a frame index where we'd normally expect a
// register.
assert(MI.getNumOperands() >= 2 && "All AArch64 cmps should have 2 operands");
if (!MI.getOperand(1).isReg())
return false;

auto NormalizeCmpValue = [](int64_t Value) -> int {
// Comparison immediates may be 64-bit, but CmpValue is only an int.
// Normalize to 0/1/2 return value, where 2 indicates any value apart from
// 0 or 1.
// TODO: Switch CmpValue to int64_t in the API to avoid this.
if (Value == 0 || Value == 1)
return Value;
return 2;
};

switch (MI.getOpcode()) {
default:
break;
Expand Down Expand Up @@ -1165,7 +1155,7 @@ bool AArch64InstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
SrcReg = MI.getOperand(1).getReg();
SrcReg2 = 0;
CmpMask = ~0;
CmpValue = NormalizeCmpValue(MI.getOperand(2).getImm());
CmpValue = MI.getOperand(2).getImm();
return true;
case AArch64::ANDSWri:
case AArch64::ANDSXri:
Expand All @@ -1174,9 +1164,9 @@ bool AArch64InstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
SrcReg = MI.getOperand(1).getReg();
SrcReg2 = 0;
CmpMask = ~0;
CmpValue = NormalizeCmpValue(AArch64_AM::decodeLogicalImmediate(
CmpValue = AArch64_AM::decodeLogicalImmediate(
MI.getOperand(2).getImm(),
MI.getOpcode() == AArch64::ANDSWri ? 32 : 64));
MI.getOpcode() == AArch64::ANDSWri ? 32 : 64);
return true;
}

Expand Down Expand Up @@ -1437,8 +1427,8 @@ bool AArch64InstrInfo::optimizePTestInstr(
/// instruction.
/// Only comparison with zero is supported.
bool AArch64InstrInfo::optimizeCompareInstr(
MachineInstr &CmpInstr, Register SrcReg, Register SrcReg2, int CmpMask,
int CmpValue, const MachineRegisterInfo *MRI) const {
MachineInstr &CmpInstr, Register SrcReg, Register SrcReg2, int64_t CmpMask,
int64_t CmpValue, const MachineRegisterInfo *MRI) const {
assert(CmpInstr.getParent());
assert(MRI);

Expand Down Expand Up @@ -1466,9 +1456,6 @@ bool AArch64InstrInfo::optimizeCompareInstr(
if (CmpInstr.getOpcode() == AArch64::PTEST_PP)
return optimizePTestInstr(&CmpInstr, SrcReg, SrcReg2, MRI);

// Warning: CmpValue == 2 indicates *any* value apart from 0 or 1.
assert((CmpValue == 0 || CmpValue == 1 || CmpValue == 2) &&
"CmpValue must be 0, 1, or 2!");
if (SrcReg2 != 0)
return false;

Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/Target/AArch64/AArch64InstrInfo.h
Expand Up @@ -227,12 +227,12 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo {
/// in SrcReg and SrcReg2, and the value it compares against in CmpValue.
/// Return true if the comparison instruction can be analyzed.
bool analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &CmpMask,
int &CmpValue) const override;
Register &SrcReg2, int64_t &CmpMask,
int64_t &CmpValue) const override;
/// optimizeCompareInstr - Convert the instruction supplying the argument to
/// the comparison into one that sets the zero bit in the flags register.
bool optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
Register SrcReg2, int CmpMask, int CmpValue,
Register SrcReg2, int64_t CmpMask, int64_t CmpValue,
const MachineRegisterInfo *MRI) const override;
bool optimizeCondBranch(MachineInstr &MI) const override;

Expand Down
13 changes: 7 additions & 6 deletions llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
Expand Up @@ -2798,8 +2798,8 @@ bool llvm::rewriteARMFrameIndex(MachineInstr &MI, unsigned FrameRegIdx,
/// compares against in CmpValue. Return true if the comparison instruction
/// can be analyzed.
bool ARMBaseInstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &CmpMask,
int &CmpValue) const {
Register &SrcReg2, int64_t &CmpMask,
int64_t &CmpValue) const {
switch (MI.getOpcode()) {
default: break;
case ARM::CMPri:
Expand Down Expand Up @@ -2870,7 +2870,8 @@ inline static ARMCC::CondCodes getCmpToAddCondition(ARMCC::CondCodes CC) {
/// This function can be extended later on.
inline static bool isRedundantFlagInstr(const MachineInstr *CmpI,
Register SrcReg, Register SrcReg2,
int ImmValue, const MachineInstr *OI,
int64_t ImmValue,
const MachineInstr *OI,
bool &IsThumb1) {
if ((CmpI->getOpcode() == ARM::CMPrr || CmpI->getOpcode() == ARM::t2CMPrr) &&
(OI->getOpcode() == ARM::SUBrr || OI->getOpcode() == ARM::t2SUBrr) &&
Expand Down Expand Up @@ -3005,8 +3006,8 @@ static bool isOptimizeCompareCandidate(MachineInstr *MI, bool &IsThumb1) {
/// operands are swapped: SUBrr(r1,r2) and CMPrr(r2,r1), by updating the
/// condition code of instructions which use the flags.
bool ARMBaseInstrInfo::optimizeCompareInstr(
MachineInstr &CmpInstr, Register SrcReg, Register SrcReg2, int CmpMask,
int CmpValue, const MachineRegisterInfo *MRI) const {
MachineInstr &CmpInstr, Register SrcReg, Register SrcReg2, int64_t CmpMask,
int64_t CmpValue, const MachineRegisterInfo *MRI) const {
// Get the unique definition of SrcReg.
MachineInstr *MI = MRI->getUniqueVRegDef(SrcReg);
if (!MI) return false;
Expand Down Expand Up @@ -3293,7 +3294,7 @@ bool ARMBaseInstrInfo::shouldSink(const MachineInstr &MI) const {
MachineBasicBlock::const_iterator Next = &MI;
++Next;
Register SrcReg, SrcReg2;
int CmpMask, CmpValue;
int64_t CmpMask, CmpValue;
bool IsThumb1;
if (Next != MI.getParent()->end() &&
analyzeCompare(*Next, SrcReg, SrcReg2, CmpMask, CmpValue) &&
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/Target/ARM/ARMBaseInstrInfo.h
Expand Up @@ -289,15 +289,15 @@ class ARMBaseInstrInfo : public ARMGenInstrInfo {
/// compares against in CmpValue. Return true if the comparison instruction
/// can be analyzed.
bool analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &CmpMask,
int &CmpValue) const override;
Register &SrcReg2, int64_t &CmpMask,
int64_t &CmpValue) const override;

/// optimizeCompareInstr - Convert the instruction to set the zero flag so
/// that we can remove a "comparison with zero"; Remove a redundant CMP
/// instruction if the flags can be updated in the same way by an earlier
/// instruction such as SUB.
bool optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
Register SrcReg2, int CmpMask, int CmpValue,
Register SrcReg2, int64_t CmpMask, int64_t CmpValue,
const MachineRegisterInfo *MRI) const override;

bool analyzeSelect(const MachineInstr &MI,
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/Target/Hexagon/HexagonHardwareLoops.cpp
Expand Up @@ -468,7 +468,7 @@ bool HexagonHardwareLoops::findInductionRegister(MachineLoop *L,
return false;

Register CmpReg1, CmpReg2;
int CmpImm = 0, CmpMask = 0;
int64_t CmpImm = 0, CmpMask = 0;
bool CmpAnalyzed =
TII->analyzeCompare(*PredI, CmpReg1, CmpReg2, CmpMask, CmpImm);
// Fail if the compare was not analyzed, or it's not comparing a register
Expand Down Expand Up @@ -652,7 +652,7 @@ CountValue *HexagonHardwareLoops::getLoopTripCount(MachineLoop *L,
unsigned CondOpc = CondI->getOpcode();

Register CmpReg1, CmpReg2;
int Mask = 0, ImmValue = 0;
int64_t Mask = 0, ImmValue = 0;
bool AnalyzedCmp =
TII->analyzeCompare(*CondI, CmpReg1, CmpReg2, Mask, ImmValue);
if (!AnalyzedCmp)
Expand Down Expand Up @@ -1453,7 +1453,7 @@ bool HexagonHardwareLoops::loopCountMayWrapOrUnderFlow(
E = MRI->use_instr_nodbg_end(); I != E; ++I) {
MachineInstr *MI = &*I;
Register CmpReg1, CmpReg2;
int CmpMask = 0, CmpValue = 0;
int64_t CmpMask = 0, CmpValue = 0;

if (!TII->analyzeCompare(*MI, CmpReg1, CmpReg2, CmpMask, CmpValue))
continue;
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/Hexagon/HexagonInstrInfo.cpp
Expand Up @@ -1791,8 +1791,8 @@ HexagonInstrInfo::CreateTargetPostRAHazardRecognizer(
/// compares against in CmpValue. Return true if the comparison instruction
/// can be analyzed.
bool HexagonInstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &Mask,
int &Value) const {
Register &SrcReg2, int64_t &Mask,
int64_t &Value) const {
unsigned Opc = MI.getOpcode();

// Set mask and the first source register.
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/Hexagon/HexagonInstrInfo.h
Expand Up @@ -270,7 +270,8 @@ class HexagonInstrInfo : public HexagonGenInstrInfo {
/// compares against in CmpValue. Return true if the comparison instruction
/// can be analyzed.
bool analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &Mask, int &Value) const override;
Register &SrcReg2, int64_t &Mask,
int64_t &Value) const override;

/// Compute the instruction latency of a given instruction.
/// If the instruction has higher cost when predicated, it's returned via
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/Hexagon/HexagonSplitDouble.cpp
Expand Up @@ -508,7 +508,7 @@ void HexagonSplitDoubleRegs::collectIndRegsForLoop(const MachineLoop *L,
while (CmpI->getOpcode() == Hexagon::C2_not)
CmpI = MRI->getVRegDef(CmpI->getOperand(1).getReg());

int Mask = 0, Val = 0;
int64_t Mask = 0, Val = 0;
bool OkCI = TII->analyzeCompare(*CmpI, CmpR1, CmpR2, Mask, Val);
if (!OkCI)
return;
Expand Down
11 changes: 6 additions & 5 deletions llvm/lib/Target/Lanai/LanaiInstrInfo.cpp
Expand Up @@ -175,8 +175,8 @@ LanaiInstrInfo::getSerializableDirectMachineOperandTargetFlags() const {
}

bool LanaiInstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &CmpMask,
int &CmpValue) const {
Register &SrcReg2, int64_t &CmpMask,
int64_t &CmpValue) const {
switch (MI.getOpcode()) {
default:
break;
Expand All @@ -203,7 +203,7 @@ bool LanaiInstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
// * SFSUB_F_RR can be made redundant by SUB_RI if the operands are the same.
// * SFSUB_F_RI can be made redundant by SUB_I if the operands are the same.
inline static bool isRedundantFlagInstr(MachineInstr *CmpI, unsigned SrcReg,
unsigned SrcReg2, int ImmValue,
unsigned SrcReg2, int64_t ImmValue,
MachineInstr *OI) {
if (CmpI->getOpcode() == Lanai::SFSUB_F_RR &&
OI->getOpcode() == Lanai::SUB_R &&
Expand Down Expand Up @@ -281,8 +281,9 @@ inline static unsigned flagSettingOpcodeVariant(unsigned OldOpcode) {
}

bool LanaiInstrInfo::optimizeCompareInstr(
MachineInstr &CmpInstr, Register SrcReg, Register SrcReg2, int /*CmpMask*/,
int CmpValue, const MachineRegisterInfo *MRI) const {
MachineInstr &CmpInstr, Register SrcReg, Register SrcReg2,
int64_t /*CmpMask*/, int64_t CmpValue,
const MachineRegisterInfo *MRI) const {
// Get the unique definition of SrcReg.
MachineInstr *MI = MRI->getUniqueVRegDef(SrcReg);
if (!MI)
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/Target/Lanai/LanaiInstrInfo.h
Expand Up @@ -96,14 +96,14 @@ class LanaiInstrInfo : public LanaiGenInstrInfo {
// SrcReg2 if having two register operands, and the value it compares against
// in CmpValue. Return true if the comparison instruction can be analyzed.
bool analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &CmpMask,
int &CmpValue) const override;
Register &SrcReg2, int64_t &CmpMask,
int64_t &CmpValue) const override;

// See if the comparison instruction can be converted into something more
// efficient. E.g., on Lanai register-register instructions can set the flag
// register, obviating the need for a separate compare.
bool optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
Register SrcReg2, int CmpMask, int CmpValue,
Register SrcReg2, int64_t CmpMask, int64_t CmpValue,
const MachineRegisterInfo *MRI) const override;

// Analyze the given select instruction, returning true if it cannot be
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
Expand Up @@ -2343,8 +2343,8 @@ bool PPCInstrInfo::ClobbersPredicate(MachineInstr &MI,
}

bool PPCInstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &Mask,
int &Value) const {
Register &SrcReg2, int64_t &Mask,
int64_t &Value) const {
unsigned Opc = MI.getOpcode();

switch (Opc) {
Expand Down Expand Up @@ -2373,7 +2373,8 @@ bool PPCInstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
}

bool PPCInstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
Register SrcReg2, int Mask, int Value,
Register SrcReg2, int64_t Mask,
int64_t Value,
const MachineRegisterInfo *MRI) const {
if (DisableCmpOpt)
return false;
Expand Down
5 changes: 3 additions & 2 deletions llvm/lib/Target/PowerPC/PPCInstrInfo.h
Expand Up @@ -524,10 +524,11 @@ class PPCInstrInfo : public PPCGenInstrInfo {
// Comparison optimization.

bool analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &Mask, int &Value) const override;
Register &SrcReg2, int64_t &Mask,
int64_t &Value) const override;

bool optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
Register SrcReg2, int Mask, int Value,
Register SrcReg2, int64_t Mask, int64_t Value,
const MachineRegisterInfo *MRI) const override;


Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp
Expand Up @@ -514,8 +514,8 @@ unsigned SystemZInstrInfo::insertBranch(MachineBasicBlock &MBB,
}

bool SystemZInstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &Mask,
int &Value) const {
Register &SrcReg2, int64_t &Mask,
int64_t &Value) const {
assert(MI.isCompare() && "Caller should have checked for a comparison");

if (MI.getNumExplicitOperands() == 2 && MI.getOperand(0).isReg() &&
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/SystemZ/SystemZInstrInfo.h
Expand Up @@ -234,7 +234,8 @@ class SystemZInstrInfo : public SystemZGenInstrInfo {
const DebugLoc &DL,
int *BytesAdded = nullptr) const override;
bool analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &Mask, int &Value) const override;
Register &SrcReg2, int64_t &Mask,
int64_t &Value) const override;
bool canInsertSelect(const MachineBasicBlock &, ArrayRef<MachineOperand> Cond,
Register, Register, Register, int &, int &,
int &) const override;
Expand Down
10 changes: 5 additions & 5 deletions llvm/lib/Target/X86/X86InstrInfo.cpp
Expand Up @@ -3922,8 +3922,8 @@ void X86InstrInfo::loadRegFromStackSlot(MachineBasicBlock &MBB,
}

bool X86InstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &CmpMask,
int &CmpValue) const {
Register &SrcReg2, int64_t &CmpMask,
int64_t &CmpValue) const {
switch (MI.getOpcode()) {
default: break;
case X86::CMP64ri32:
Expand Down Expand Up @@ -4010,7 +4010,7 @@ bool X86InstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
/// ImmValue: immediate for FlagI if it takes an immediate.
inline static bool isRedundantFlagInstr(const MachineInstr &FlagI,
Register SrcReg, Register SrcReg2,
int ImmMask, int ImmValue,
int64_t ImmMask, int64_t ImmValue,
const MachineInstr &OI) {
if (((FlagI.getOpcode() == X86::CMP64rr && OI.getOpcode() == X86::SUB64rr) ||
(FlagI.getOpcode() == X86::CMP32rr && OI.getOpcode() == X86::SUB32rr) ||
Expand Down Expand Up @@ -4207,8 +4207,8 @@ static X86::CondCode isUseDefConvertible(const MachineInstr &MI) {
/// operates on the same source operands and sets flags in the same way as
/// Compare; remove Compare if possible.
bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
Register SrcReg2, int CmpMask,
int CmpValue,
Register SrcReg2, int64_t CmpMask,
int64_t CmpValue,
const MachineRegisterInfo *MRI) const {
// Check whether we can replace SUB with CMP.
switch (CmpInstr.getOpcode()) {
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/Target/X86/X86InstrInfo.h
Expand Up @@ -510,14 +510,14 @@ class X86InstrInfo final : public X86GenInstrInfo {
/// compares against in CmpValue. Return true if the comparison instruction
/// can be analyzed.
bool analyzeCompare(const MachineInstr &MI, Register &SrcReg,
Register &SrcReg2, int &CmpMask,
int &CmpValue) const override;
Register &SrcReg2, int64_t &CmpMask,
int64_t &CmpValue) const override;

/// optimizeCompareInstr - Check if there exists an earlier instruction that
/// operates on the same source operands and sets flags in the same way as
/// Compare; remove Compare if possible.
bool optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
Register SrcReg2, int CmpMask, int CmpValue,
Register SrcReg2, int64_t CmpMask, int64_t CmpValue,
const MachineRegisterInfo *MRI) const override;

/// optimizeLoadInstr - Try to remove the load by folding it to a register
Expand Down

0 comments on commit 0529e2e

Please sign in to comment.