[PATCH] D34952: [ARM] Adjust ifcvt heuristic for the diamond ifcvt case

[Sam Parker via Phabricator via llvm-commits]

samparker accepted this revision.
samparker added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lib/Target/ARM/ARMBaseInstrInfo.cpp:1885
+      // discount it from PredCost.
+      PredCost -= 1 * ScalingUpFactor;
     }
----------------
john.brawn wrote:
> samparker wrote:
> > 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?
> 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.
ok, sounds like a reasonable estimation then.


================
Comment at: test/CodeGen/Thumb2/ifcvt-no-branch-predictor.ll:98
 ; 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
----------------
john.brawn wrote:
> samparker wrote:
> > why does this patch affect targets with a branch predictor?
> 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).
ah! sorry, missed that.


Repository:
  rL LLVM

https://reviews.llvm.org/D34952