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

[Joel Galenson via Phabricator via llvm-commits]

jgalenson added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:4484
+  };
+  if (LHS.getOpcode() == ISD::AND &&
+      (isOneConstant(RHS) || isNullConstant(RHS)) &&
----------------
efriedma wrote:
> 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.
Okay, thanks.  I'll look into a patch that simply adds that and then rebase this commit on top of it, which will allow me to remove the AND-checking logic here.


https://reviews.llvm.org/D35635