[PATCH] D26367: Fix DAGCombiner match

Renato Golin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 14 04:46:29 PST 2016

rengolin added a comment.

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.




More information about the llvm-commits mailing list