Skip to content

Commit 8456c3a

Browse files
committed
AArch64: fix regression introduced by fcmp immediate selection.
Forgot to check if the predicate is safe to commutate operands.
1 parent a61e42e commit 8456c3a

File tree

2 files changed

+43
-8
lines changed

2 files changed

+43
-8
lines changed

llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "llvm/CodeGen/MachineRegisterInfo.h"
3535
#include "llvm/CodeGen/TargetOpcodes.h"
3636
#include "llvm/IR/Constants.h"
37+
#include "llvm/IR/Instructions.h"
3738
#include "llvm/IR/PatternMatch.h"
3839
#include "llvm/IR/Type.h"
3940
#include "llvm/IR/IntrinsicsAArch64.h"
@@ -177,8 +178,10 @@ class AArch64InstructionSelector : public InstructionSelector {
177178
MachineIRBuilder &MIRBuilder) const;
178179

179180
/// Emit a floating point comparison between \p LHS and \p RHS.
181+
/// \p Pred if given is the intended predicate to use.
180182
MachineInstr *emitFPCompare(Register LHS, Register RHS,
181-
MachineIRBuilder &MIRBuilder) const;
183+
MachineIRBuilder &MIRBuilder,
184+
Optional<CmpInst::Predicate> = None) const;
182185

183186
MachineInstr *emitInstr(unsigned Opcode,
184187
std::initializer_list<llvm::DstOp> DstOps,
@@ -1483,11 +1486,11 @@ bool AArch64InstructionSelector::selectCompareBranchFedByFCmp(
14831486
assert(I.getOpcode() == TargetOpcode::G_BRCOND);
14841487
// Unfortunately, the mapping of LLVM FP CC's onto AArch64 CC's isn't
14851488
// totally clean. Some of them require two branches to implement.
1486-
emitFPCompare(FCmp.getOperand(2).getReg(), FCmp.getOperand(3).getReg(), MIB);
1489+
auto Pred = (CmpInst::Predicate)FCmp.getOperand(1).getPredicate();
1490+
emitFPCompare(FCmp.getOperand(2).getReg(), FCmp.getOperand(3).getReg(), MIB,
1491+
Pred);
14871492
AArch64CC::CondCode CC1, CC2;
1488-
changeFCMPPredToAArch64CC(
1489-
static_cast<CmpInst::Predicate>(FCmp.getOperand(1).getPredicate()), CC1,
1490-
CC2);
1493+
changeFCMPPredToAArch64CC(static_cast<CmpInst::Predicate>(Pred), CC1, CC2);
14911494
MachineBasicBlock *DestMBB = I.getOperand(1).getMBB();
14921495
MIB.buildInstr(AArch64::Bcc, {}, {}).addImm(CC1).addMBB(DestMBB);
14931496
if (CC2 != AArch64CC::AL)
@@ -3090,7 +3093,7 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
30903093
CmpInst::Predicate Pred =
30913094
static_cast<CmpInst::Predicate>(I.getOperand(1).getPredicate());
30923095
if (!emitFPCompare(I.getOperand(2).getReg(), I.getOperand(3).getReg(),
3093-
MIRBuilder) ||
3096+
MIRBuilder, Pred) ||
30943097
!emitCSetForFCmp(I.getOperand(0).getReg(), Pred, MIRBuilder))
30953098
return false;
30963099
I.eraseFromParent();
@@ -4211,7 +4214,8 @@ MachineInstr *AArch64InstructionSelector::emitCSetForFCmp(
42114214

42124215
MachineInstr *
42134216
AArch64InstructionSelector::emitFPCompare(Register LHS, Register RHS,
4214-
MachineIRBuilder &MIRBuilder) const {
4217+
MachineIRBuilder &MIRBuilder,
4218+
Optional<CmpInst::Predicate> Pred) const {
42154219
MachineRegisterInfo &MRI = *MIRBuilder.getMRI();
42164220
LLT Ty = MRI.getType(LHS);
42174221
if (Ty.isVector())
@@ -4224,7 +4228,12 @@ AArch64InstructionSelector::emitFPCompare(Register LHS, Register RHS,
42244228
// to explicitly materialize a constant.
42254229
const ConstantFP *FPImm = getConstantFPVRegVal(RHS, MRI);
42264230
bool ShouldUseImm = FPImm && (FPImm->isZero() && !FPImm->isNegative());
4227-
if (!ShouldUseImm) {
4231+
4232+
auto IsEqualityPred = [](CmpInst::Predicate P) {
4233+
return P == CmpInst::FCMP_OEQ || P == CmpInst::FCMP_ONE ||
4234+
P == CmpInst::FCMP_UEQ || P == CmpInst::FCMP_UNE;
4235+
};
4236+
if (!ShouldUseImm && Pred && IsEqualityPred(*Pred)) {
42284237
// Try commutating the operands.
42294238
const ConstantFP *LHSImm = getConstantFPVRegVal(LHS, MRI);
42304239
if (LHSImm && (LHSImm->isZero() && !LHSImm->isNegative())) {

llvm/test/CodeGen/AArch64/GlobalISel/select-fcmp.mir

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,3 +134,29 @@ body: |
134134
RET_ReallyLR implicit $s0
135135
136136
...
137+
---
138+
name: zero_lhs_not_commutative_pred
139+
alignment: 4
140+
legalized: true
141+
regBankSelected: true
142+
tracksRegLiveness: true
143+
body: |
144+
bb.1:
145+
liveins: $s0, $s1
146+
147+
; CHECK-LABEL: name: zero_lhs_not_commutative_pred
148+
; CHECK: liveins: $s0, $s1
149+
; CHECK: [[COPY:%[0-9]+]]:fpr32 = COPY $s0
150+
; CHECK: [[FMOVS0_:%[0-9]+]]:fpr32 = FMOVS0
151+
; CHECK: FCMPSrr [[FMOVS0_]], [[COPY]], implicit-def $nzcv
152+
; CHECK: [[CSINCWr:%[0-9]+]]:gpr32 = CSINCWr $wzr, $wzr, 5, implicit $nzcv
153+
; CHECK: $s0 = COPY [[CSINCWr]]
154+
; CHECK: RET_ReallyLR implicit $s0
155+
%0:fpr(s32) = COPY $s0
156+
%1:fpr(s32) = COPY $s1
157+
%2:fpr(s32) = G_FCONSTANT float 0.000000e+00
158+
%3:gpr(s32) = G_FCMP floatpred(olt), %2(s32), %0
159+
$s0 = COPY %3(s32)
160+
RET_ReallyLR implicit $s0
161+
162+
...

0 commit comments

Comments
 (0)