[PATCH] D26367: Fix DAGCombiner match

James Molloy via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 14 11:29:08 PST 2016


jmolloy added a comment.

In https://reviews.llvm.org/D26367#622049, @rengolin wrote:

> In https://reviews.llvm.org/D26367#620246, @evstupac wrote:
>
> > Today the behavior is the same 2 byte loads. So there is no regressions. I'm just unable to add new test for ARM.
>
>
> Right, I have just confirmed this behaviour. Also, your patch improves AArch64 (as you said), so I think the best course of action now is to:
>
> - add the same snippet above to the ARM test, checking for two `ldrb`s and with a big `FIXME` saying that we need to fix PartialOffset in the DAGCombiner to make it work.
> - Copy that test to the AArch64 directory, change the triple to "aarch64-linux-gnu" and change the register patterns from r[0-9] to w[0-9].
> - Change the last (added) test in AArch64 to expect `ldrh` and see it pass.
>
>   I can help you with the tests, but the error messages should be reasonably simple to match.
>
>   James, given that this is not changing the existing behaviour on ARM and is improving x86 and AArch64 code (and didn't show variation in spec), it should be ok to merge.
>
>   cheers, --renato


Ok, LGTM from my side.


Repository:
  rL LLVM

https://reviews.llvm.org/D26367





More information about the llvm-commits mailing list