This teaches AArch64TargetLowering::shouldSinkOperands to sink the operands of aarch64_neon_pmull intrinsic.
Thanks for the patch! Can the test be stripped down? Does there need an actual loop or would just 2 different basic blocks be enough? I realise the test of a related patch is also similar, but ideally we would have more compact tests.
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.
Can you run update_llc_test_checks on the file. It's useful in cases like this to show all the output
I think you can likely remove this too.
IIUC there's still a test case missing to guard against the original issue mentioned by @dmgreen.
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
You could use -asm-verbose=0 to trim down the output a bit.
it's slightly easier to read if there's a newline after the terminators.
Looks like a good patch, from what I can see. Thanks
You don't need brackets around single statements in llvm. I would personally also invert the condition - if (thing) doathing() seems clearer in intent to me, but I do see that other cases here work the other way arounds already.