[PATCH] D67350: [IfCvt][ARM] Optimise diamond if-conversion for code size

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/IfConversion.cpp:292
+      const MachineFunction &MF = *TBBInfo.BB->getParent();
+      if (MF.getFunction().hasOptSize()) {
+        MachineBasicBlock::iterator TIB = TBBInfo.BB->begin();
----------------
Maybe minsize?  I don't think we want to predicate some ridiculous number of instructions to save a few bytes at -Os.


================
Comment at: llvm/lib/CodeGen/IfConversion.cpp:309
+        // When if-converting a diamond, the branches at the ends of all the
+        // true block, false block and common predecessor will all be removed.
+        for (auto &I : make_range(TIE, TBBInfo.BB->end())) {
----------------
Is this actually true, if the terminator isn't analyzable?  We can form a "diamond" where TrueBB and FalseBB have an arbitrary terminator, like an INLINEASM_BR, as long as it's the same.


================
Comment at: llvm/lib/CodeGen/IfConversion.cpp:337
+        // their code size.
+        CommonBytes /= 2;
+
----------------
Are you adding CommonBytes for common instructions before the predicated ones, somewhere?


================
Comment at: llvm/lib/CodeGen/IfConversion.cpp:985
+  TrueBBICalc.IsBrAnalyzable = TrueBBI.IsBrAnalyzable;
+  FalseBBICalc.IsBrAnalyzable = FalseBBI.IsBrAnalyzable;
   if (!RescanInstructions(TIB, FIB, TIE, FIE, TrueBBICalc, FalseBBICalc))
----------------
This looks weird; why are we constructing TrueBBICalc/FalseBBICalc like this?  What does this change do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67350