[PATCH] D120230: [SelectOpti][1/4] Setup new select-optimize pass

[Sotiris Apostolakis via Phabricator via llvm-commits]

apostolakis added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/CodeGenPassBuilder.h:673
+  // Convert conditional moves to conditional jumps when profitable.
+  if (getOptLevel() != CodeGenOpt::None && !Opt.DisableSelectOptimize)
+    addPass(SelectOptimizePass());
----------------
davidxl wrote:
> Do we need a canonicalization phase before this to convert jumps to select when possible?
SimplifyCFG canonicalizes jumps to selects when possible. Currently SimplifyCFG prevents this canonicalization for a few cases where it is deemed very unprofitable. These cases seem reasonable and are probably not worth converting back to selects just before this pass and reconsidering; although it might be worth empirically evaluating the effect of that. What is more important in my opinion is for SimplifyCFG to canonicalize to selects without trying to optimize to maximize its enabling effect for subsequent LLVM IR optimizations (some preliminary experiments showed a small benefit from such a change). Then at the end, this new pass will decide what form is more profitable. This matter was briefly discussed recently in https://reviews.llvm.org/D118066. Once this new pass is enabled for all PGO builds, I will advocate for changes in SimplifyCFG to aggressively canonicalize to selects when profile information are available and an informed decision can be made later without blocking any LLVM IR optimizations. 


================
Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:933
+  // Convert conditional moves to conditional jumps when profitable.
+  if (getOptLevel() != CodeGenOpt::None && !DisableSelectOptimize)
+    addPass(createSelectOptimizePass());
----------------
davidxl wrote:
> Not suitable when size optimization is on.
The Code generation optimization level that is checked here (llvm::CodeGenOpt::Level) only contains 4 levels (None, Less, Default, Aggressive), none of which is meant for optimize-for-size. I also do not see any other API at this level that would enable a check for size. Instead, the check for size-opts right now is happening in the beginning of the new pass (see the next patch in this series: D120231), where it checks the attributes of the function for OptSize and also invokes the shouldOptimizeForSize function that uses PGSO heuristics. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120230/new/

https://reviews.llvm.org/D120230