[PATCH] D59208: [DAGCombiner] fold (addcarry (xor a, -1), b, c) -> (subcarry b, a, !c) and flip carry.

Wed Jul 3 16:03:14 PDT 2019

deadalnix marked an inline comment as done.
deadalnix added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2711
+                                  const TargetLowering &TLI,
+                                  bool Force = true) {
+  if (Force && isa<ConstantSDNode>(V))
----------------
deadalnix wrote:
> RKSimon wrote:
> > Are there any callers that use Force == false?
> I have to be honest here, that patch is from march and my memory isn't super fresh. Reading the code, this seems wrong and I'm surprised things work at all. When force is set to true, then extractBooleanFlip will `ALWAYS` flip the boolean, even if that means adding new ops. Clearly, this is not what is intended in the select case for instance. It's somewhat surprising that no test broke. I'll investigate further and see if I can come up with a test case for select.
After further investigation, it turns out that select use a boolean predicate, so `Force` cannot have any effect (1 is already recognized as a flip, 0 is a noop and a constant gets optimized away).

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59208