[PATCH] D117944: [AARCH64][NEON] Allow to sink operands for aarch64_neon_pmull.

[Sunho Kim via Phabricator via llvm-commits]

sunho marked 2 inline comments as done.
sunho added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12109
       }
       LLVM_FALLTHROUGH;
 
----------------
fhahn wrote:
> dmgreen wrote:
> > This falls through to trying to treat the instruction like an indexed pmul, which isn't an instruction available in AArch64. This could do with a test case and possibly being guarded against.
> IIUC there's still a test case missing to guard against the original issue mentioned by @dmgreen. 
I've added a test case that makes sure splat vector is not sunk for pmull in "sink-free-instructions.ll."


================
Comment at: llvm/test/CodeGen/AArch64/neon-vmull-high-p8.ll:8
+
+define <8 x i16> @test_pmull2_sink(<16 x i8> %a, <16 x i8> %b, <8 x i16> %c, i1 %t) {
+entry:
----------------
fhahn wrote:
> sunho wrote:
> > fhahn wrote:
> > > it would also be good to add a few negative tests, where sinking won't happen, e.g. because there are users in other blocks. Also cases where the other or both shuffles need sinking.
> > When would sinking not happen? I've read the code, but it seems it's always sinking unless the operand is phi nod .
> You are right, users in different BBs do not limit sinking, because the instructions are duplicated in that case. One case where sinking won't happen is if one operand has a high mask and one a low mask for example. But it might be better to test for something like that in `llvm/test/Transforms/CodeGenPrepare/AArch64/sink-free-instructions.ll`
I've added some negative tests in the file you mentioned.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117944/new/

https://reviews.llvm.org/D117944