[PATCH] D35635: Optimize {s,u}{add,sub}.with.overflow on ARM

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:4484
+  };
+  if (LHS.getOpcode() == ISD::AND &&
+      (isOneConstant(RHS) || isNullConstant(RHS)) &&
----------------
jgalenson wrote:
> efriedma wrote:
> > jgalenson wrote:
> > > efriedma wrote:
> > > > jgalenson wrote:
> > > > > efriedma wrote:
> > > > > > We should be able to optimize away the AND here using known bits; does that not happen on trunk?
> > > > > It doesn't because ARM doesn't call setBooleanContents.  If there's a way to set that correctly, it would indeed be better.  Do you think we could do that?
> > > > Really?  Wow, that's a big oversight.  We should fix that.
> > > Do you know the correct way to set it?  I tried, but a few tests failed, and I don't know the architecture well enough to know if that was my fault or of the tests just needed to be tweaked.
> > > 
> > > If we do make that change, then I'll be able to simplify this code, of course.
> > It's likely the tests just need to be tweaked; a lot of them are pretty sensitive to the exact compiler output.
> So which argument to setBooleanContents is correct for ARM?  Is it ZeroOrOne?
ZeroOrOne is probably best; among other things, it matches the AAPCS calling convention rules for bool.


https://reviews.llvm.org/D35635