[ARM] Adjust ifcvt heuristic for the diamond ifcvt case

[john.brawn]

Summary

When we have a diamond ifcvt the fallthough block will have a branch at the end of it that disappears when predicated, so discount it from the predication cost.

[john.brawn]

Ping.

[samparker]

lib/Target/ARM/ARMBaseInstrInfo.cpp

1885 ↗

(On Diff #105087)

Does the '1' represent the single branch instruction that we will not have to execute? Does it not have to be the number of cycles saved?

test/CodeGen/Thumb2/ifcvt-no-branch-predictor.ll

98 ↗

(On Diff #105087)

why does this patch affect targets with a branch predictor?

[john.brawn]

lib/Target/ARM/ARMBaseInstrInfo.cpp

1885 ↗

(On Diff #105087)

IfConversion sets the cost of an instruction to be 1 + (latency-1) if latency > 1, but looking at the various sched models latency of branches is either 0 or 1 as a branch doesn't produce any result. Also getting the latency of an instruction requires having a TargetSchedModel which isn't available here, so I'd have to plumb it in from IfConversion.

test/CodeGen/Thumb2/ifcvt-no-branch-predictor.ll

98 ↗

(On Diff #105087)

It doesn't, I've adjusted the function (see the extra store in if.then below) to make sure we see a difference between bp and nobp (because with the new heuristic both produced the same code for the current version of this function).

[samparker]

lib/Target/ARM/ARMBaseInstrInfo.cpp

1885 ↗

(On Diff #105087)

ok, sounds like a reasonable estimation then.

test/CodeGen/Thumb2/ifcvt-no-branch-predictor.ll

98 ↗

(On Diff #105087)

ah! sorry, missed that.

llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.cpp

if (!Subtarget.hasBranchPredictor()) {
     if (!FCycles) {
       // Triangle: TBB is the fallthrough
       TUnpredCycles = TCycles + NotTakenBranchCost;
       FUnpredCycles = TakenBranchCost;
     } else {
       // Diamond: TBB is the block that is branched to, FBB is the fallthrough
       TUnpredCycles = TCycles + TakenBranchCost;
       FUnpredCycles = FCycles + NotTakenBranchCost;
+      // The branch at the end of FBB will disappear when it's predicated, so
+      // discount it from PredCost.
+      PredCost -= 1 * ScalingUpFactor;
     }
     // The total cost is the cost of each path scaled by their probabilites
     unsigned TUnpredCost = Probability.scale(TUnpredCycles * ScalingUpFactor);
     unsigned FUnpredCost = Probability.getCompl().scale(FUnpredCycles * ScalingUpFactor);
     UnpredCost = TUnpredCost + FUnpredCost;
     // When predicating assume that the first IT can be folded away but later
     // ones cost one cycle each
     if (Subtarget.isThumb2() && TCycles + FCycles > 4) {

llvm/trunk/test/CodeGen/Thumb2/ifcvt-no-branch-predictor.ll

 
 if.end:
   %n.addr.0 = phi i32 [ %n, %if.then ], [ %0, %if.else ]
   tail call void @otherfn()
   ret i32 %n.addr.0
 }
 
 ; CHECK-LABEL: diamond2:
-; CHECK-BP: itte
-; CHECK-BP: streq
-; CHECK-BP: ldreq
-; CHECK-BP: strne
-; CHECK-NOBP: cbz
-; CHECK-NOBP: str
-; CHECK-NOBP: b
-; CHECK-NOBP: str
-; CHECK-NOBP: ldr
+; CHECK-BP: cbz
+; CHECK-BP: str
+; CHECK-BP: str
+; CHECK-BP: b
+; CHECK-BP: str
+; CHECK-BP: ldr
+; CHECK-NOBP: ittee
+; CHECK-NOBP: streq
+; CHECK-NOBP: ldreq
+; CHECK-NOBP: strne
+; CHECK-NOBP: strne
 define i32 @diamond2(i32 %n, i32 %m, i32* %p, i32* %q) {
 entry:
   %tobool = icmp eq i32 %n, 0
   br i1 %tobool, label %if.else, label %if.then
 
 if.then:
   store i32 %n, i32* %p, align 4
+  %arrayidx = getelementptr inbounds i32, i32* %p, i32 2
+  store i32 %n, i32* %arrayidx, align 4
   br label %if.end
 
 if.else:
   store i32 %m, i32* %q, align 4
   %0 = load i32, i32* %p, align 4
   br label %if.end
 
 if.end: