[AArch64] CondOpt pass is missing FCMPSri and FCMPDri

[zzheng]

Summary

We need to bail out on FCMPSri and FCMPDri when searching backward for a CMP which defines the flags used by B.CC.

This patch fixes the following false positive case:

cmp w19, #0
cinc w0, w19, gt
...
fcmp d8, #0.0
b.gt .LBB0_5

[sdmitrouk]

Hi,

again my bad, sorry about that. Please see one inline comment.

Thanks for fixing this!

Regards,
Sergey

lib/Target/AArch64/AArch64ConditionOptimizer.cpp

165

I checked FPComparison multiclass from AArch64InstrFormats.td and it creates 4 forms of an instruction. I missed ri forms for both FCMP and FCMPE, which means that two more cases should be added here as well:

case AArch64::FCMPEDri:
case AArch64::FCMPESri:

[mcrosier]

Hi Zhaoshi,
Assuming you address my comments as well as handle all four cases (FCMPDri, FCMPSri, FCMPEDri, FCMPESri) this LGTM.

Chad

test/CodeGen/AArch64/combine-comparisons-by-cse.ll

361

Please add : after the function name.

Explicitly,

; CHECK-LABEL: fcmpri:

385

Please remove 'tail' from the call site.

Multiple instances in the test case.

[mcrosier]

Committed r220961.

lib/Target/AArch64/AArch64ConditionOptimizer.cpp

     case AArch64::ADDSXri:
       return I;
 
+    // Prevent false positive case like:
+    // cmp      w19, #0
+    // cinc     w0, w19, gt
+    // ...
+    // fcmp     d8, #0.0
+    // b.gt     .LBB0_5
+    case AArch64::FCMPDri:
+    case AArch64::FCMPSri:
+
     case AArch64::SUBSWrr:
     case AArch64::SUBSXrr:
     case AArch64::ADDSWrr:

test/CodeGen/AArch64/combine-comparisons-by-cse.ll

   %retval.0 = phi i32 [ 0, %if.end ], [ 123, %land.lhs.true ]
   ret i32 %retval.0
 }
+
+
+; Test in the following case, we don't hit 'cmp' and trigger a false positive
+; cmp  w19, #0
+; cinc w0, w19, gt
+; ...
+; fcmp d8, #0.0
+; b.gt .LBB0_5
+
+define i32 @fcmpri(i32 %argc, i8** nocapture readonly %argv) {
+
+; CHECK-LABEL: fcmpri
+; CHECK: cmp w0, #2
+; CHECK: b.lt .LBB9_3
+; CHECK-NOT: cmp w0, #1
+; CHECK-NOT: b.le .LBB9_3
+
+; CHECK-LABEL-DAG: .LBB9_3
+; CHECK: cmp w19, #0
+; CHECK: fcmp d8, #0.0
+; CHECK: b.gt .LBB9_5
+; CHECK-NOT: cmp w19, #1
+; CHECK-NOT: b.ge .LBB9_5
+
+entry:
+  %cmp = icmp sgt i32 %argc, 1
+  br i1 %cmp, label %land.lhs.true, label %if.end
+
+land.lhs.true:                                    ; preds = %entry
+  %arrayidx = getelementptr inbounds i8** %argv, i64 1
+  %0 = load i8** %arrayidx, align 8
+  %cmp1 = icmp eq i8* %0, null
+  br i1 %cmp1, label %if.end, label %return
+
+if.end:                                           ; preds = %land.lhs.true, %entry
+  %call = tail call i32 @zoo(i32 1)
+  %call2 = tail call double @yoo(i32 -1)
+  %cmp4 = icmp sgt i32 %call, 0
+  %add = zext i1 %cmp4 to i32
+  %cond = add nsw i32 %add, %call
+  %call7 = tail call i32 @xoo(i32 %cond, i32 2)
+  %cmp9 = fcmp ogt double %call2, 0.000000e+00
+  br i1 %cmp9, label %cond.end14, label %cond.false12
+
+cond.false12:                                     ; preds = %if.end
+  %sub = fadd fast double %call2, -1.000000e+00
+  br label %cond.end14
+
+cond.end14:                                       ; preds = %if.end, %cond.false12
+  %cond15 = phi double [ %sub, %cond.false12 ], [ %call2, %if.end ]
+  %call16 = tail call i32 @woo(double %cond15, double -2.000000e+00)
+  br label %return
+
+return:                                           ; preds = %land.lhs.true, %cond.end14
+  %retval.0 = phi i32 [ 4, %cond.end14 ], [ 3, %land.lhs.true ]
+  ret i32 %retval.0
+}
+
+declare i32 @zoo(i32)
+
+declare double @yoo(i32)
+
+declare i32 @xoo(i32, i32)
+
+declare i32 @woo(double, double)