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")
{
