[PATCH] D139981: [EarlyIfConversion] Add switch to allow for multiple ifcvt iterations.

Wed Dec 14 08:44:58 PST 2022

jroelofs added inline comments.

================
Comment at: llvm/lib/CodeGen/EarlyIfConversion.cpp:57
+// This bypasses all other heuristics, so it should be set fairly high.
+static cl::opt<bool> AllowPredicatingTwice(
+    "allow-predicating-twice", cl::init(false), cl::Hidden,
----------------
hgreving wrote:
> arsenm wrote:
> > A command line flag isn’t the way to expose this. I would move this into a partner bit with isPredicable, maybe combining into a predicable kind field 
> Thanks for the comment. The reason (I agree) I just added this switch is that it is a property / tweak of the pass, and only applies to archs where a predicable instruction "is predicated". On Hexagon, a predicted instruction (AFAICS, had to dig in more) is _not_ predicable. On x86, an instruction never returns isPredicated (I guess this makes sense).  ARM/AArch64 not 100% sure.
> 
> WDYT about a TII->canPredicatePredicatedIntrs() hook? Reason being this is more a property of the arch being able to support predicate logic rather than a property of each intruction.
> 
> Also problem is which architecture could I test this with :(?
A TII hook seems appropriate. How about `TII->canPredicatePredicatedInstr(*I)`, so the interface doesn't need to change again when a target comes along that needs to decide per-instruction.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139981