[PATCH] D32776: Enable branch coalescing on PowerPC

[Eric Christopher via Phabricator via llvm-commits]

echristo requested changes to this revision.
echristo added a comment.
This revision now requires changes to proceed.

Hrm.

I can think of three ways to enable this:

a) Enable it all the time, this would take some benchmarking across platforms, but I think is the right long term direction.
b) Add it as a pass in the backend, and use something like a lambda passed into the constructor to conditionalize it if you want (see inline question below)
c) Do this for now, but do all of the work in a as well to turn it on (i.e. a above) and remove the subtarget query (caveat of the inline question below).

I guess this comes down to my (now expanded) inline question of:

why don't we just want this on all the time on all targets?

One additional followup question:

Did you see any other performance changes across spec/lnt? Any degredations? Any other benchmark runs?

Thanks!

-eric



================
Comment at: lib/CodeGen/TargetPassConfig.cpp:722
   // Coalesce basic blocks with the same branch condition
-  addPass(&BranchCoalescingID);
+  if (getOptLevel() != CodeGenOpt::None)
+    addPass(&BranchCoalescingID);
----------------
You don't need this because of the skipFunction query in the pass.


================
Comment at: lib/Target/PowerPC/PPCSubtarget.cpp:183
+bool PPCSubtarget::enableBranchCoalescing() const {
+  return (isSVR4ABI() && isPPC64());
+}
----------------
Is this the right choice or is this just the only place it's been tested?


https://reviews.llvm.org/D32776