[PATCH] D140931: Improve and enable folding of conditional branches with tail calls.

Noah Goldstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 4 00:11:15 PST 2023


goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:3018-3019
     assert(BranchCond.size() == 1);
+    // At the moment MayChangeDirection is unused (or always treated as true).
+    // We only match existing CC.
     if (CC != BranchCond[0].getImm())
----------------
pengfei wrote:
> We do we need to add it if it's unused? Do not put unrelated/useless code in the same patch. It obstructs people to understand the patch.
Its not needed. The reason I added it was b.c the rationale previously for keeping this optimization guarded behind `-Os` was that if it changed branch direction + reordered CFG it could be a de-optimization. That seems like a reasonable concern so thought it would be good to have in the generic API.

A more appropriate comment might have been: "At the moment never change branch direction".

Can drop if your prefer. Let me know for V2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140931



More information about the llvm-commits mailing list