On 05/09/17 18:48, Bernd Edlinger wrote:
On 09/05/17 17:02, Wilco Dijkstra wrote:
Bernd Edlinger wrote:
Combine creates an invalid insn out of these two insns:
Yes it looks like a latent bug. We need to use arm_general_register_operand
as arm_adddi3/subdi3 only allow integer registers. You don't need a new
predicate
s_register_operand_nv. Also I'd prefer something like arm_general_adddi_operand.
Thanks, attached is a patch following your suggestion.
+ "TARGET_32BIT && ((!TARGET_NEON && !TARGET_IWMMXT) || reload_completed)"
The split condition for adddi3 now looks more accurate indeed, although we could
remove the !TARGET_NEON from the split condition as this is always true given
arm_adddi3 uses "TARGET_32BIT && !TARGET_NEON".
No, the split condition does not begin with "&& TARGET_32BIT...".
Therefore the split is enabled in TARGET_NEON after reload_completed.
And it is invoked from adddi3_neon for all alternatives without vfp
registers:
switch (which_alternative)
{
case 0: /* fall through */
case 3: return "vadd.i64\t%P0, %P1, %P2";
case 1: return "#";
case 2: return "#";
case 4: return "#";
case 5: return "#";
case 6: return "#";
Also there are more cases, a quick grep suggests *anddi_notdi_di has the same
issue.
Yes, that pattern can be cleaned up in a follow-up patch.
Note this splitter is invoked from bicdi3_neon as well.
However I think anddi_notdi_di should be safe as long as it is enabled
after reload_completed (which is probably a bug).
Thanks, that's what I had in mind in my other reply.
This is ok if testing comes back ok.
Kyrill
Bernd.
Wilco
