[PATCH] D62774: [DAGCombine][X86][AArch64][MIPS][LANAI] (C - x) - y -> C - (x + y) fold (PR41952)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked 4 inline comments as done.
lebedev.ri added a comment.

Thank you for the review!



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:3017
   }
+  // (C - x) - y  ->  C - (x + y)
+  if (N0.hasOneUse() && N0.getOpcode() == ISD::SUB &&
----------------
RKSimon wrote:
> lebedev.ri wrote:
> > RKSimon wrote:
> > > Do we gain anything from the general case here? This seems to cause a small increase in register pressure in many cases - limit to the (0 - x) - y negation case only?
> > Could you please be more specific?
> > The only regressions i'm seeing are 32-bit `@reg64_lshr_by_sub_from_negated*`
> > The obvious positives are all these newly-formed `neg`
> > https://godbolt.org/z/JK0ldm <- which is kinda the endgoal here.
> > 
> > If we limit this to only hoist `neg` (and this will be the first such restriction
> > in all of these patches), then naturally, the `@reg64_lshr_by_sub_from_negated*`
> > pattern will no longer produce `neg`, since the `sub 64, %x` will not be sunk.
> > 
> > That can still be fixed, if we transform `C-X` to `neg(x)+C` or so in DAGCombine,
> > I was actually going to look into that, but it looked to not be necessary
> > with this patch in it's current form.
> Yes I think your right - sorry I was being over cautious! 
No problem, thank you for confirming my understanding here!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62774