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). Bernd. > Wilco >
2017-09-05 Bernd Edlinger <bernd.edlin...@hotmail.de> PR target/77308 * config/arm/predicates.md (arm_general_adddi_operand): Create new non-vfp predicate. * config/arm/arm.md (*arm_adddi3, *arm_subdi3): Use new predicates. Index: gcc/config/arm/arm.md =================================================================== --- gcc/config/arm/arm.md (revision 251663) +++ gcc/config/arm/arm.md (working copy) @@ -457,14 +457,13 @@ ) (define_insn_and_split "*arm_adddi3" - [(set (match_operand:DI 0 "s_register_operand" "=&r,&r,&r,&r,&r") - (plus:DI (match_operand:DI 1 "s_register_operand" "%0, 0, r, 0, r") - (match_operand:DI 2 "arm_adddi_operand" "r, 0, r, Dd, Dd"))) + [(set (match_operand:DI 0 "arm_general_register_operand" "=&r,&r,&r,&r,&r") + (plus:DI (match_operand:DI 1 "arm_general_register_operand" "%0, 0, r, 0, r") + (match_operand:DI 2 "arm_general_adddi_operand" "r, 0, r, Dd, Dd"))) (clobber (reg:CC CC_REGNUM))] "TARGET_32BIT && !TARGET_NEON" "#" - "TARGET_32BIT && ((!TARGET_NEON && !TARGET_IWMMXT) || reload_completed) - && ! (TARGET_NEON && IS_VFP_REGNUM (REGNO (operands[0])))" + "TARGET_32BIT && ((!TARGET_NEON && !TARGET_IWMMXT) || reload_completed)" [(parallel [(set (reg:CC_C CC_REGNUM) (compare:CC_C (plus:SI (match_dup 1) (match_dup 2)) (match_dup 1))) @@ -1263,9 +1262,9 @@ ) (define_insn_and_split "*arm_subdi3" - [(set (match_operand:DI 0 "s_register_operand" "=&r,&r,&r") - (minus:DI (match_operand:DI 1 "s_register_operand" "0,r,0") - (match_operand:DI 2 "s_register_operand" "r,0,0"))) + [(set (match_operand:DI 0 "arm_general_register_operand" "=&r,&r,&r") + (minus:DI (match_operand:DI 1 "arm_general_register_operand" "0,r,0") + (match_operand:DI 2 "arm_general_register_operand" "r,0,0"))) (clobber (reg:CC CC_REGNUM))] "TARGET_32BIT && !TARGET_NEON" "#" ; "subs\\t%Q0, %Q1, %Q2\;sbc\\t%R0, %R1, %R2" Index: gcc/config/arm/predicates.md =================================================================== --- gcc/config/arm/predicates.md (revision 251663) +++ gcc/config/arm/predicates.md (working copy) @@ -82,6 +82,11 @@ || REGNO (op) >= FIRST_PSEUDO_REGISTER)); }) +(define_predicate "arm_general_adddi_operand" + (ior (match_operand 0 "arm_general_register_operand") + (and (match_code "const_int") + (match_test "const_ok_for_dimode_op (INTVAL (op), PLUS)")))) + (define_predicate "vfp_register_operand" (match_code "reg,subreg") {