[PATCH] D36151: [GlobalISel] Only merge memory ops for mayLoad or mayStore instrs.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D36151#827515, @fhahn wrote:

> Yes, https://reviews.llvm.org/D36094 fixes the problem too! Here's what's going on in the test case, without any of the 2 patches:


Great! In that case I'll update the summary of that patch to indicate that arm-isel.ll shows the same problem in-tree.

> Would it still make sense to only emit GIR_MergeMemOperands for instructions that mayLoad/mayStore? Unless I am missing something, I think not having this action when it's not required would speed up GlobalISel a tiny bit.

Yes, it does. As you say, not using GIR_MergeMemOperands unnecessarily will speed up the matcher. It will also become important later on when load/store is importable and we support multi-instruction emission since something will have to ensure the memory operands are added to appropriate instructions.

This patch LGTM. Ideally, https://reviews.llvm.org/D36094 would land first since that fixes the underlying problem but that hasn't been reviewed yet and I see no reason to require that it lands first.


https://reviews.llvm.org/D36151