[PATCH] D35375: [AArch64][Atomic] Canonicalize sub of immediate to add of -immediate.

[Christof Douma via Phabricator via llvm-commits]

christof added a comment.

In https://reviews.llvm.org/D35375#808895, @gberry wrote:

> In https://reviews.llvm.org/D35375#808771, @mcrosier wrote:
>
> > In https://reviews.llvm.org/D35375#808725, @gberry wrote:
> >
> > > Perhaps this would be better done at the IR level, in InstCombine?
> >
> >
> > Seems reasonable, but we'd have to know this is a good canonicalization for all targets, right?
>
>
> It seems like it is to me.  Worst case (e.g. the target supports 'add' but not 'sub'), the sub would ideally just be transformed back and a negate added during ISel legalization.


Surely it depends on the immediate range and add/sub which are supported by the target whether or not it is good for that target? Negative immediate might be harder for some targets, I would expect. I won't be surprised if some code generators give a regression if that normalisation is done in general. They might generate worse code if they see `add -1` instead of `sub 1`. Just my two cents.

I am not really well known with all the tricks you can do with tablegen, but I would think this particular rewrite could be done with a pattern match in tablegen, could it not? I think there is already something for `add -imm` to `sub imm`.


https://reviews.llvm.org/D35375