* [PATCH] Improve arm and aarch64 casesi (PR target/70341)
@ 2019-02-23 0:29 Jakub Jelinek
2019-02-25 10:23 ` Kyrill Tkachov
2019-02-27 17:22 ` James Greenhalgh
0 siblings, 2 replies; 9+ messages in thread
From: Jakub Jelinek @ 2019-02-23 0:29 UTC (permalink / raw)
To: Richard Earnshaw, Ramana Radhakrishnan, Kyrylo Tkachov, James Greenhalgh
Cc: gcc-patches
Hi!
The testcase in the PR doesn't hoist any memory loads from the large switch
before the switch on aarch64 and arm (unlike e.g. x86), because the
arm/aarch64 casesi patterns don't properly annotate the memory load from the
jump table. It is created by gen_* and in RTL directly one can't specify
the needed flags (MEM_READONLY_P and MEM_NOTRAP_P).
Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi and
aarch64-linux, ok for trunk?
2019-02-23 Jakub Jelinek <jakub@redhat.com>
PR target/70341
* config/arm/arm.md (arm_casesi_internal): New define_expand. Rename
old define_insn to ...
(*arm_casesi_internal): ... this. Add mode to LABEL_REFs.
* config/arm/thumb2.md (thumb2_casesi_internal): New define_expand.
Rename old define_insn to ...
(*thumb2_casesi_internal): ... this. Add mode to LABEL_REFs.
(thumb2_casesi_internal_pic): New define_expand. Rename old
define_insn to ...
(*thumb2_casesi_internal_pic): ... this. Add mode to LABEL_REFs.
* config/aarch64/aarch64.md (casesi): Create the casesi_dispatch
MEM manually here, set MEM_READONLY_P and MEM_NOTRAP_P on it.
--- gcc/config/arm/arm.md.jj 2019-02-18 20:48:32.643732307 +0100
+++ gcc/config/arm/arm.md 2019-02-21 14:40:50.603452028 +0100
@@ -8914,16 +8914,35 @@ (define_expand "casesi"
;; The USE in this pattern is needed to tell flow analysis that this is
;; a CASESI insn. It has no other purpose.
-(define_insn "arm_casesi_internal"
+(define_expand "arm_casesi_internal"
+ [(parallel [(set (pc)
+ (if_then_else
+ (leu (match_operand:SI 0 "s_register_operand")
+ (match_operand:SI 1 "arm_rhs_operand"))
+ (match_dup 4)
+ (label_ref:SI (match_operand 3 ""))))
+ (clobber (reg:CC CC_REGNUM))
+ (use (label_ref:SI (match_operand 2 "")))])]
+ "TARGET_ARM"
+{
+ operands[4] = gen_rtx_MULT (SImode, operands[0], GEN_INT (4));
+ operands[4] = gen_rtx_PLUS (SImode, operands[4],
+ gen_rtx_LABEL_REF (SImode, operands[2]));
+ operands[4] = gen_rtx_MEM (SImode, operands[4]);
+ MEM_READONLY_P (operands[4]) = 1;
+ MEM_NOTRAP_P (operands[4]) = 1;
+})
+
+(define_insn "*arm_casesi_internal"
[(parallel [(set (pc)
(if_then_else
(leu (match_operand:SI 0 "s_register_operand" "r")
(match_operand:SI 1 "arm_rhs_operand" "rI"))
(mem:SI (plus:SI (mult:SI (match_dup 0) (const_int 4))
- (label_ref (match_operand 2 "" ""))))
- (label_ref (match_operand 3 "" ""))))
+ (label_ref:SI (match_operand 2 "" ""))))
+ (label_ref:SI (match_operand 3 "" ""))))
(clobber (reg:CC CC_REGNUM))
- (use (label_ref (match_dup 2)))])]
+ (use (label_ref:SI (match_dup 2)))])]
"TARGET_ARM"
"*
if (flag_pic)
--- gcc/config/arm/thumb2.md.jj 2019-01-01 12:37:28.280792453 +0100
+++ gcc/config/arm/thumb2.md 2019-02-21 15:00:26.811137210 +0100
@@ -1079,17 +1079,37 @@ (define_insn "thumb2_zero_extendqisi2_v6
(set_attr "neg_pool_range" "*,250")]
)
-(define_insn "thumb2_casesi_internal"
+(define_expand "thumb2_casesi_internal"
+ [(parallel [(set (pc)
+ (if_then_else
+ (leu (match_operand:SI 0 "s_register_operand")
+ (match_operand:SI 1 "arm_rhs_operand"))
+ (match_dup 4)
+ (label_ref:SI (match_operand 3 ""))))
+ (clobber (reg:CC CC_REGNUM))
+ (clobber (match_scratch:SI 5))
+ (use (label_ref:SI (match_operand 2 "")))])]
+ "TARGET_THUMB2 && !flag_pic"
+{
+ operands[4] = gen_rtx_MULT (SImode, operands[0], GEN_INT (4));
+ operands[4] = gen_rtx_PLUS (SImode, operands[4],
+ gen_rtx_LABEL_REF (SImode, operands[2]));
+ operands[4] = gen_rtx_MEM (SImode, operands[4]);
+ MEM_READONLY_P (operands[4]) = 1;
+ MEM_NOTRAP_P (operands[4]) = 1;
+})
+
+(define_insn "*thumb2_casesi_internal"
[(parallel [(set (pc)
(if_then_else
(leu (match_operand:SI 0 "s_register_operand" "r")
(match_operand:SI 1 "arm_rhs_operand" "rI"))
(mem:SI (plus:SI (mult:SI (match_dup 0) (const_int 4))
- (label_ref (match_operand 2 "" ""))))
- (label_ref (match_operand 3 "" ""))))
+ (label_ref:SI (match_operand 2 "" ""))))
+ (label_ref:SI (match_operand 3 "" ""))))
(clobber (reg:CC CC_REGNUM))
(clobber (match_scratch:SI 4 "=&r"))
- (use (label_ref (match_dup 2)))])]
+ (use (label_ref:SI (match_dup 2)))])]
"TARGET_THUMB2 && !flag_pic"
"* return thumb2_output_casesi(operands);"
[(set_attr "conds" "clob")
@@ -1097,18 +1117,39 @@ (define_insn "thumb2_casesi_internal"
(set_attr "type" "multiple")]
)
-(define_insn "thumb2_casesi_internal_pic"
+(define_expand "thumb2_casesi_internal_pic"
+ [(parallel [(set (pc)
+ (if_then_else
+ (leu (match_operand:SI 0 "s_register_operand")
+ (match_operand:SI 1 "arm_rhs_operand"))
+ (match_dup 4)
+ (label_ref:SI (match_operand 3 ""))))
+ (clobber (reg:CC CC_REGNUM))
+ (clobber (match_scratch:SI 5))
+ (clobber (match_scratch:SI 6))
+ (use (label_ref:SI (match_operand 2 "")))])]
+ "TARGET_THUMB2 && flag_pic"
+{
+ operands[4] = gen_rtx_MULT (SImode, operands[0], GEN_INT (4));
+ operands[4] = gen_rtx_PLUS (SImode, operands[4],
+ gen_rtx_LABEL_REF (SImode, operands[2]));
+ operands[4] = gen_rtx_MEM (SImode, operands[4]);
+ MEM_READONLY_P (operands[4]) = 1;
+ MEM_NOTRAP_P (operands[4]) = 1;
+})
+
+(define_insn "*thumb2_casesi_internal_pic"
[(parallel [(set (pc)
(if_then_else
(leu (match_operand:SI 0 "s_register_operand" "r")
(match_operand:SI 1 "arm_rhs_operand" "rI"))
(mem:SI (plus:SI (mult:SI (match_dup 0) (const_int 4))
- (label_ref (match_operand 2 "" ""))))
- (label_ref (match_operand 3 "" ""))))
+ (label_ref:SI (match_operand 2 "" ""))))
+ (label_ref:SI (match_operand 3 "" ""))))
(clobber (reg:CC CC_REGNUM))
(clobber (match_scratch:SI 4 "=&r"))
(clobber (match_scratch:SI 5 "=r"))
- (use (label_ref (match_dup 2)))])]
+ (use (label_ref:SI (match_dup 2)))])]
"TARGET_THUMB2 && flag_pic"
"* return thumb2_output_casesi(operands);"
[(set_attr "conds" "clob")
--- gcc/config/aarch64/aarch64.md.jj 2019-01-19 09:39:18.847831222 +0100
+++ gcc/config/aarch64/aarch64.md 2019-02-21 15:25:27.874532191 +0100
@@ -622,13 +622,27 @@ (define_expand "casesi"
operands[0], operands[2], operands[4]));
operands[2] = force_reg (DImode, gen_rtx_LABEL_REF (DImode, operands[3]));
- emit_jump_insn (gen_casesi_dispatch (operands[2], operands[0],
- operands[3]));
+ operands[2]
+ = gen_rtx_UNSPEC (Pmode, gen_rtvec (2, operands[2], operands[0]),
+ UNSPEC_CASESI);
+ operands[2] = gen_rtx_MEM (DImode, operands[2]);
+ MEM_READONLY_P (operands[2]) = 1;
+ MEM_NOTRAP_P (operands[2]) = 1;
+ emit_jump_insn (gen_casesi_dispatch (operands[2], operands[3]));
DONE;
}
)
-(define_insn "casesi_dispatch"
+(define_expand "casesi_dispatch"
+ [(parallel
+ [(set (pc) (match_operand:DI 0 ""))
+ (clobber (reg:CC CC_REGNUM))
+ (clobber (match_scratch:DI 2))
+ (clobber (match_scratch:DI 3))
+ (use (label_ref:DI (match_operand 1 "")))])]
+ "")
+
+(define_insn "*casesi_dispatch"
[(parallel
[(set (pc)
(mem:DI (unspec [(match_operand:DI 0 "register_operand" "r")
@@ -637,7 +651,7 @@ (define_insn "casesi_dispatch"
(clobber (reg:CC CC_REGNUM))
(clobber (match_scratch:DI 3 "=r"))
(clobber (match_scratch:DI 4 "=r"))
- (use (label_ref (match_operand 2 "" "")))])]
+ (use (label_ref:DI (match_operand 2 "" "")))])]
""
"*
return aarch64_output_casesi (operands);
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Improve arm and aarch64 casesi (PR target/70341)
2019-02-23 0:29 [PATCH] Improve arm and aarch64 casesi (PR target/70341) Jakub Jelinek
@ 2019-02-25 10:23 ` Kyrill Tkachov
2019-02-25 10:27 ` Jakub Jelinek
2019-02-27 17:22 ` James Greenhalgh
1 sibling, 1 reply; 9+ messages in thread
From: Kyrill Tkachov @ 2019-02-25 10:23 UTC (permalink / raw)
To: Jakub Jelinek, Richard Earnshaw, Ramana Radhakrishnan, James Greenhalgh
Cc: gcc-patches
Hi Jakub,
On 2/23/19 12:20 AM, Jakub Jelinek wrote:
> Hi!
>
> The testcase in the PR doesn't hoist any memory loads from the large
> switch
> before the switch on aarch64 and arm (unlike e.g. x86), because the
> arm/aarch64 casesi patterns don't properly annotate the memory load
> from the
> jump table. It is created by gen_* and in RTL directly one can't specify
> the needed flags (MEM_READONLY_P and MEM_NOTRAP_P).
>
> Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi and
> aarch64-linux, ok for trunk?
>
Since you're changing Arm and Thumb-2-specific paths can you please make
sure to bootstrap configurations --with-mode=arm and --with-mode=thumb.
Otherwise the arm parts (and aarch64 FWIW) look ok to me. Most of the
patch content is splitting the define_insn into a define_expand so that
we can annotate the MEM with the right attributes.
Thanks,
Kyrill
> 2019-02-23 Jakub Jelinek <jakub@redhat.com>
>
> Â Â Â Â Â Â Â PR target/70341
> Â Â Â Â Â Â Â * config/arm/arm.md (arm_casesi_internal): New define_expand.Â
> Rename
> Â Â Â Â Â Â Â old define_insn to ...
>        (*arm_casesi_internal): ... this. Add mode to LABEL_REFs.
> Â Â Â Â Â Â Â * config/arm/thumb2.md (thumb2_casesi_internal): New
> define_expand.
> Â Â Â Â Â Â Â Rename old define_insn to ...
>        (*thumb2_casesi_internal): ... this. Add mode to LABEL_REFs.
> Â Â Â Â Â Â Â (thumb2_casesi_internal_pic): New define_expand. Rename old
> Â Â Â Â Â Â Â define_insn to ...
>        (*thumb2_casesi_internal_pic): ... this. Add mode to LABEL_REFs.
> Â Â Â Â Â Â Â * config/aarch64/aarch64.md (casesi): Create the casesi_dispatch
> Â Â Â Â Â Â Â MEM manually here, set MEM_READONLY_P and MEM_NOTRAP_P on it.
>
> --- gcc/config/arm/arm.md.jj   2019-02-18 20:48:32.643732307 +0100
> +++ gcc/config/arm/arm.md      2019-02-21 14:40:50.603452028 +0100
> @@ -8914,16 +8914,35 @@ (define_expand "casesi"
>
> Â ;; The USE in this pattern is needed to tell flow analysis that this is
>  ;; a CASESI insn. It has no other purpose.
> -(define_insn "arm_casesi_internal"
> +(define_expand "arm_casesi_internal"
> +Â [(parallel [(set (pc)
> +Â Â Â Â Â Â Â Â Â Â Â Â Â (if_then_else
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (leu (match_operand:SI 0 "s_register_operand")
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:SI 1 "arm_rhs_operand"))
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_dup 4)
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref:SI (match_operand 3 ""))))
> +Â Â Â Â Â Â Â Â Â Â Â Â (clobber (reg:CC CC_REGNUM))
> +Â Â Â Â Â Â Â Â Â Â Â Â (use (label_ref:SI (match_operand 2 "")))])]
> +Â "TARGET_ARM"
> +{
> +Â operands[4] = gen_rtx_MULT (SImode, operands[0], GEN_INT (4));
> +Â operands[4] = gen_rtx_PLUS (SImode, operands[4],
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â gen_rtx_LABEL_REF (SImode, operands[2]));
> +Â operands[4] = gen_rtx_MEM (SImode, operands[4]);
> +Â MEM_READONLY_P (operands[4]) = 1;
> +Â MEM_NOTRAP_P (operands[4]) = 1;
> +})
> +
> +(define_insn "*arm_casesi_internal"
> Â Â [(parallel [(set (pc)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (if_then_else
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (leu (match_operand:SI 0 "s_register_operand" "r")
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:SI 1 "arm_rhs_operand" "rI"))
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (mem:SI (plus:SI (mult:SI (match_dup 0) (const_int 4))
> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref (match_operand 2 "" ""))))
> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref (match_operand 3 "" ""))))
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref:SI (match_operand 2 "" ""))))
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref:SI (match_operand 3 "" ""))))
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â (clobber (reg:CC CC_REGNUM))
> -Â Â Â Â Â Â Â Â Â Â Â Â (use (label_ref (match_dup 2)))])]
> +Â Â Â Â Â Â Â Â Â Â Â Â (use (label_ref:SI (match_dup 2)))])]
> Â Â "TARGET_ARM"
> Â Â "*
> Â Â Â Â if (flag_pic)
> --- gcc/config/arm/thumb2.md.jj 2019-01-01 12:37:28.280792453 +0100
> +++ gcc/config/arm/thumb2.md   2019-02-21 15:00:26.811137210 +0100
> @@ -1079,17 +1079,37 @@ (define_insn "thumb2_zero_extendqisi2_v6
> Â Â Â (set_attr "neg_pool_range" "*,250")]
> Â )
>
> -(define_insn "thumb2_casesi_internal"
> +(define_expand "thumb2_casesi_internal"
> +Â [(parallel [(set (pc)
> +Â Â Â Â Â Â Â Â Â Â Â Â Â (if_then_else
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (leu (match_operand:SI 0 "s_register_operand")
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:SI 1 "arm_rhs_operand"))
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_dup 4)
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref:SI (match_operand 3 ""))))
> +Â Â Â Â Â Â Â Â Â Â Â Â (clobber (reg:CC CC_REGNUM))
> +Â Â Â Â Â Â Â Â Â Â Â Â (clobber (match_scratch:SI 5))
> +Â Â Â Â Â Â Â Â Â Â Â Â (use (label_ref:SI (match_operand 2 "")))])]
> +Â "TARGET_THUMB2 && !flag_pic"
> +{
> +Â operands[4] = gen_rtx_MULT (SImode, operands[0], GEN_INT (4));
> +Â operands[4] = gen_rtx_PLUS (SImode, operands[4],
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â gen_rtx_LABEL_REF (SImode, operands[2]));
> +Â operands[4] = gen_rtx_MEM (SImode, operands[4]);
> +Â MEM_READONLY_P (operands[4]) = 1;
> +Â MEM_NOTRAP_P (operands[4]) = 1;
> +})
> +
> +(define_insn "*thumb2_casesi_internal"
> Â Â [(parallel [(set (pc)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (if_then_else
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (leu (match_operand:SI 0 "s_register_operand" "r")
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:SI 1 "arm_rhs_operand" "rI"))
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (mem:SI (plus:SI (mult:SI (match_dup 0) (const_int 4))
> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref (match_operand 2 "" ""))))
> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref (match_operand 3 "" ""))))
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref:SI (match_operand 2 "" ""))))
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref:SI (match_operand 3 "" ""))))
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â (clobber (reg:CC CC_REGNUM))
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â (clobber (match_scratch:SI 4 "=&r"))
> -Â Â Â Â Â Â Â Â Â Â Â Â (use (label_ref (match_dup 2)))])]
> +Â Â Â Â Â Â Â Â Â Â Â Â (use (label_ref:SI (match_dup 2)))])]
> Â Â "TARGET_THUMB2 && !flag_pic"
> Â Â "* return thumb2_output_casesi(operands);"
> Â Â [(set_attr "conds" "clob")
> @@ -1097,18 +1117,39 @@ (define_insn "thumb2_casesi_internal"
> Â Â Â (set_attr "type" "multiple")]
> Â )
>
> -(define_insn "thumb2_casesi_internal_pic"
> +(define_expand "thumb2_casesi_internal_pic"
> +Â [(parallel [(set (pc)
> +Â Â Â Â Â Â Â Â Â Â Â Â Â (if_then_else
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (leu (match_operand:SI 0 "s_register_operand")
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:SI 1 "arm_rhs_operand"))
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_dup 4)
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref:SI (match_operand 3 ""))))
> +Â Â Â Â Â Â Â Â Â Â Â Â (clobber (reg:CC CC_REGNUM))
> +Â Â Â Â Â Â Â Â Â Â Â Â (clobber (match_scratch:SI 5))
> +Â Â Â Â Â Â Â Â Â Â Â Â (clobber (match_scratch:SI 6))
> +Â Â Â Â Â Â Â Â Â Â Â Â (use (label_ref:SI (match_operand 2 "")))])]
> +Â "TARGET_THUMB2 && flag_pic"
> +{
> +Â operands[4] = gen_rtx_MULT (SImode, operands[0], GEN_INT (4));
> +Â operands[4] = gen_rtx_PLUS (SImode, operands[4],
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â gen_rtx_LABEL_REF (SImode, operands[2]));
> +Â operands[4] = gen_rtx_MEM (SImode, operands[4]);
> +Â MEM_READONLY_P (operands[4]) = 1;
> +Â MEM_NOTRAP_P (operands[4]) = 1;
> +})
> +
> +(define_insn "*thumb2_casesi_internal_pic"
> Â Â [(parallel [(set (pc)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (if_then_else
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (leu (match_operand:SI 0 "s_register_operand" "r")
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:SI 1 "arm_rhs_operand" "rI"))
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (mem:SI (plus:SI (mult:SI (match_dup 0) (const_int 4))
> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref (match_operand 2 "" ""))))
> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref (match_operand 3 "" ""))))
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref:SI (match_operand 2 "" ""))))
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (label_ref:SI (match_operand 3 "" ""))))
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â (clobber (reg:CC CC_REGNUM))
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â (clobber (match_scratch:SI 4 "=&r"))
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â (clobber (match_scratch:SI 5 "=r"))
> -Â Â Â Â Â Â Â Â Â Â Â Â (use (label_ref (match_dup 2)))])]
> +Â Â Â Â Â Â Â Â Â Â Â Â (use (label_ref:SI (match_dup 2)))])]
> Â Â "TARGET_THUMB2 && flag_pic"
> Â Â "* return thumb2_output_casesi(operands);"
> Â Â [(set_attr "conds" "clob")
> --- gcc/config/aarch64/aarch64.md.jj   2019-01-19 09:39:18.847831222
> +0100
> +++ gcc/config/aarch64/aarch64.md      2019-02-21 15:25:27.874532191
> +0100
> @@ -622,13 +622,27 @@ (define_expand "casesi"
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â operands[0], operands[2],
> operands[4]));
>
> Â Â Â Â operands[2] = force_reg (DImode, gen_rtx_LABEL_REF (DImode,
> operands[3]));
> -Â Â Â emit_jump_insn (gen_casesi_dispatch (operands[2], operands[0],
> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â operands[3]));
> +Â Â Â operands[2]
> +Â Â Â Â Â = gen_rtx_UNSPEC (Pmode, gen_rtvec (2, operands[2], operands[0]),
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â UNSPEC_CASESI);
> +Â Â Â operands[2] = gen_rtx_MEM (DImode, operands[2]);
> +Â Â Â MEM_READONLY_P (operands[2]) = 1;
> +Â Â Â MEM_NOTRAP_P (operands[2]) = 1;
> +Â Â Â emit_jump_insn (gen_casesi_dispatch (operands[2], operands[3]));
> Â Â Â Â DONE;
> Â Â }
> Â )
>
> -(define_insn "casesi_dispatch"
> +(define_expand "casesi_dispatch"
> +Â [(parallel
> +Â Â Â [(set (pc) (match_operand:DI 0 ""))
> +Â Â Â Â (clobber (reg:CC CC_REGNUM))
> +Â Â Â Â (clobber (match_scratch:DI 2))
> +Â Â Â Â (clobber (match_scratch:DI 3))
> +Â Â Â Â (use (label_ref:DI (match_operand 1 "")))])]
> +Â "")
> +
> +(define_insn "*casesi_dispatch"
> Â Â [(parallel
> Â Â Â Â [(set (pc)
> Â Â Â Â Â Â Â Â Â Â (mem:DI (unspec [(match_operand:DI 0 "register_operand" "r")
> @@ -637,7 +651,7 @@ (define_insn "casesi_dispatch"
> Â Â Â Â Â (clobber (reg:CC CC_REGNUM))
> Â Â Â Â Â (clobber (match_scratch:DI 3 "=r"))
> Â Â Â Â Â (clobber (match_scratch:DI 4 "=r"))
> -Â Â Â Â (use (label_ref (match_operand 2 "" "")))])]
> +Â Â Â Â (use (label_ref:DI (match_operand 2 "" "")))])]
> Â Â ""
> Â Â "*
> Â Â return aarch64_output_casesi (operands);
>
> Â Â Â Â Â Â Â Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Improve arm and aarch64 casesi (PR target/70341)
2019-02-25 10:23 ` Kyrill Tkachov
@ 2019-02-25 10:27 ` Jakub Jelinek
2019-02-25 10:29 ` Kyrill Tkachov
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2019-02-25 10:27 UTC (permalink / raw)
To: Kyrill Tkachov
Cc: Richard Earnshaw, Ramana Radhakrishnan, James Greenhalgh, gcc-patches
On Mon, Feb 25, 2019 at 10:05:46AM +0000, Kyrill Tkachov wrote:
> Hi Jakub,
>
> On 2/23/19 12:20 AM, Jakub Jelinek wrote:
> > Hi!
> >
> > The testcase in the PR doesn't hoist any memory loads from the large
> > switch
> > before the switch on aarch64 and arm (unlike e.g. x86), because the
> > arm/aarch64 casesi patterns don't properly annotate the memory load from
> > the
> > jump table. It is created by gen_* and in RTL directly one can't specify
> > the needed flags (MEM_READONLY_P and MEM_NOTRAP_P).
> >
> > Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi and
> > aarch64-linux, ok for trunk?
> >
> Since you're changing Arm and Thumb-2-specific paths can you please make
> sure to bootstrap configurations --with-mode=arm and --with-mode=thumb.
The only bootstraps I'm doing are distro builds with
--with-tune=generic-armv7-a --with-arch=armv7-a \
--with-float=hard --with-fpu=vfpv3-d16 --with-abi=aapcs-linux
I don't have setup nor experience with configuring anything else, don't
really know what is and what isn't ABI compatible etc.
Isn't --with-mode=arm the default with the above set of options? Can
--with-mode=thumb be used ABI compatibly with that, or is that incompatible?
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Improve arm and aarch64 casesi (PR target/70341)
2019-02-25 10:27 ` Jakub Jelinek
@ 2019-02-25 10:29 ` Kyrill Tkachov
2019-02-27 11:21 ` Jakub Jelinek
0 siblings, 1 reply; 9+ messages in thread
From: Kyrill Tkachov @ 2019-02-25 10:29 UTC (permalink / raw)
To: Jakub Jelinek
Cc: Richard Earnshaw, Ramana Radhakrishnan, James Greenhalgh, gcc-patches
Hi Jakub,
On 2/25/19 10:19 AM, Jakub Jelinek wrote:
> On Mon, Feb 25, 2019 at 10:05:46AM +0000, Kyrill Tkachov wrote:
>> Hi Jakub,
>>
>> On 2/23/19 12:20 AM, Jakub Jelinek wrote:
>>> Hi!
>>>
>>> The testcase in the PR doesn't hoist any memory loads from the large
>>> switch
>>> before the switch on aarch64 and arm (unlike e.g. x86), because the
>>> arm/aarch64 casesi patterns don't properly annotate the memory load from
>>> the
>>> jump table. It is created by gen_* and in RTL directly one can't specify
>>> the needed flags (MEM_READONLY_P and MEM_NOTRAP_P).
>>>
>>> Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi and
>>> aarch64-linux, ok for trunk?
>>>
>> Since you're changing Arm and Thumb-2-specific paths can you please make
>> sure to bootstrap configurations --with-mode=arm and --with-mode=thumb.
> The only bootstraps I'm doing are distro builds with
> --with-tune=generic-armv7-a --with-arch=armv7-a \
> --with-float=hard --with-fpu=vfpv3-d16 --with-abi=aapcs-linux
> I don't have setup nor experience with configuring anything else, don't
> really know what is and what isn't ABI compatible etc.
> Isn't --with-mode=arm the default with the above set of options? Can
> --with-mode=thumb be used ABI compatibly with that, or is that incompatible?
They are ABI-compatible. Running the testsuite with -mthumb in
RUNTESTFLAGS would also be enough in this case if you don't have the
cycles for a bootstrap.
Thanks,
Kyrill
>
> Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Improve arm and aarch64 casesi (PR target/70341)
2019-02-25 10:29 ` Kyrill Tkachov
@ 2019-02-27 11:21 ` Jakub Jelinek
2019-02-27 12:03 ` Richard Earnshaw (lists)
2019-02-27 15:03 ` Kyrill Tkachov
0 siblings, 2 replies; 9+ messages in thread
From: Jakub Jelinek @ 2019-02-27 11:21 UTC (permalink / raw)
To: Kyrill Tkachov
Cc: Richard Earnshaw, Ramana Radhakrishnan, James Greenhalgh, gcc-patches
On Mon, Feb 25, 2019 at 10:23:52AM +0000, Kyrill Tkachov wrote:
> > The only bootstraps I'm doing are distro builds with
> > --with-tune=generic-armv7-a --with-arch=armv7-a \
> > --with-float=hard --with-fpu=vfpv3-d16 --with-abi=aapcs-linux
> > I don't have setup nor experience with configuring anything else, don't
> > really know what is and what isn't ABI compatible etc.
> > Isn't --with-mode=arm the default with the above set of options? Can
> > --with-mode=thumb be used ABI compatibly with that, or is that incompatible?
>
>
> They are ABI-compatible. Running the testsuite with -mthumb in RUNTESTFLAGS
> would also be enough in this case if you don't have the cycles for a
> bootstrap.
Ok, so tried to do two distro builds with the above plus --with-mode=thumb,
one without the casesi patch, the other one with that.
Both bootstrapped successfully, but dunno why the regtests were too slow to
fit under our hard 2 days timeout limit. When I grabbed the build logs, the
only difference in the grep ^FAIL | sort -u lines was one fewer go failure
with the patch (but that is most likely a random failure rather than the
patch actually changing anything). Is -mthumb generally slower than ARM
mode?
Anyway, I'm afraid this is as far as I can go in my testing.
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Improve arm and aarch64 casesi (PR target/70341)
2019-02-27 11:21 ` Jakub Jelinek
@ 2019-02-27 12:03 ` Richard Earnshaw (lists)
2019-02-27 15:03 ` Kyrill Tkachov
1 sibling, 0 replies; 9+ messages in thread
From: Richard Earnshaw (lists) @ 2019-02-27 12:03 UTC (permalink / raw)
To: Jakub Jelinek, Kyrill Tkachov
Cc: Ramana Radhakrishnan, James Greenhalgh, gcc-patches
On 27/02/2019 10:56, Jakub Jelinek wrote:
> On Mon, Feb 25, 2019 at 10:23:52AM +0000, Kyrill Tkachov wrote:
>>> The only bootstraps I'm doing are distro builds with
>>> --with-tune=generic-armv7-a --with-arch=armv7-a \
>>> --with-float=hard --with-fpu=vfpv3-d16 --with-abi=aapcs-linux
>>> I don't have setup nor experience with configuring anything else, don't
>>> really know what is and what isn't ABI compatible etc.
>>> Isn't --with-mode=arm the default with the above set of options? Can
>>> --with-mode=thumb be used ABI compatibly with that, or is that incompatible?
>>
>>
>> They are ABI-compatible. Running the testsuite with -mthumb in RUNTESTFLAGS
>> would also be enough in this case if you don't have the cycles for a
>> bootstrap.
>
> Ok, so tried to do two distro builds with the above plus --with-mode=thumb,
> one without the casesi patch, the other one with that.
> Both bootstrapped successfully, but dunno why the regtests were too slow to
> fit under our hard 2 days timeout limit. When I grabbed the build logs, the
> only difference in the grep ^FAIL | sort -u lines was one fewer go failure
> with the patch (but that is most likely a random failure rather than the
> patch actually changing anything). Is -mthumb generally slower than ARM
> mode?
>
> Anyway, I'm afraid this is as far as I can go in my testing.
>
> Jakub
>
Thumb performance on v7-a should be nearly identical to Arm performance
(sometimes a bit faster, sometimes a bit slower, depending on the
precise code generated). So if you're timing out, something else is
probably going wrong.
R.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Improve arm and aarch64 casesi (PR target/70341)
2019-02-27 11:21 ` Jakub Jelinek
2019-02-27 12:03 ` Richard Earnshaw (lists)
@ 2019-02-27 15:03 ` Kyrill Tkachov
2019-02-27 15:24 ` Jakub Jelinek
1 sibling, 1 reply; 9+ messages in thread
From: Kyrill Tkachov @ 2019-02-27 15:03 UTC (permalink / raw)
To: Jakub Jelinek
Cc: Richard Earnshaw, Ramana Radhakrishnan, James Greenhalgh, gcc-patches
Hi Jakub,
On 2/27/19 10:56 AM, Jakub Jelinek wrote:
> On Mon, Feb 25, 2019 at 10:23:52AM +0000, Kyrill Tkachov wrote:
>>> The only bootstraps I'm doing are distro builds with
>>> --with-tune=generic-armv7-a --with-arch=armv7-a \
>>> --with-float=hard --with-fpu=vfpv3-d16 --with-abi=aapcs-linux
>>> I don't have setup nor experience with configuring anything else, don't
>>> really know what is and what isn't ABI compatible etc.
>>> Isn't --with-mode=arm the default with the above set of options? Can
>>> --with-mode=thumb be used ABI compatibly with that, or is that incompatible?
>>
>> They are ABI-compatible. Running the testsuite with -mthumb in RUNTESTFLAGS
>> would also be enough in this case if you don't have the cycles for a
>> bootstrap.
> Ok, so tried to do two distro builds with the above plus --with-mode=thumb,
> one without the casesi patch, the other one with that.
> Both bootstrapped successfully, but dunno why the regtests were too slow to
> fit under our hard 2 days timeout limit. When I grabbed the build logs, the
> only difference in the grep ^FAIL | sort -u lines was one fewer go failure
> with the patch (but that is most likely a random failure rather than the
> patch actually changing anything). Is -mthumb generally slower than ARM
> mode?
>
> Anyway, I'm afraid this is as far as I can go in my testing.
As discussed on IRC, I've bootstrapped and tested this on
arm-none-linux-gnueabihf.
So from an arm perspective this looks good (but you'll need an aarch64
ok for that component).
Thanks,
Kyrill
>
> Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Improve arm and aarch64 casesi (PR target/70341)
2019-02-27 15:03 ` Kyrill Tkachov
@ 2019-02-27 15:24 ` Jakub Jelinek
0 siblings, 0 replies; 9+ messages in thread
From: Jakub Jelinek @ 2019-02-27 15:24 UTC (permalink / raw)
To: Kyrill Tkachov
Cc: Richard Earnshaw, Ramana Radhakrishnan, James Greenhalgh, gcc-patches
On Wed, Feb 27, 2019 at 02:43:01PM +0000, Kyrill Tkachov wrote:
> As discussed on IRC, I've bootstrapped and tested this on
> arm-none-linux-gnueabihf.
Thanks for doing that. I've committed the config/arm/ changes, they really
don't depend on the aarch64 changes or vice versa.
> So from an arm perspective this looks good (but you'll need an aarch64 ok
> for that component).
I'll wait for review for this part.
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Improve arm and aarch64 casesi (PR target/70341)
2019-02-23 0:29 [PATCH] Improve arm and aarch64 casesi (PR target/70341) Jakub Jelinek
2019-02-25 10:23 ` Kyrill Tkachov
@ 2019-02-27 17:22 ` James Greenhalgh
1 sibling, 0 replies; 9+ messages in thread
From: James Greenhalgh @ 2019-02-27 17:22 UTC (permalink / raw)
To: Jakub Jelinek
Cc: Richard Earnshaw, Ramana Radhakrishnan, Kyrylo Tkachov, gcc-patches, nd
On Fri, Feb 22, 2019 at 06:20:51PM -0600, Jakub Jelinek wrote:
> Hi!
>
> The testcase in the PR doesn't hoist any memory loads from the large switch
> before the switch on aarch64 and arm (unlike e.g. x86), because the
> arm/aarch64 casesi patterns don't properly annotate the memory load from the
> jump table. It is created by gen_* and in RTL directly one can't specify
> the needed flags (MEM_READONLY_P and MEM_NOTRAP_P).
>
> Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi and
> aarch64-linux, ok for trunk?
>
> 2019-02-23 Jakub Jelinek <jakub@redhat.com>
>
> PR target/70341
> * config/aarch64/aarch64.md (casesi): Create the casesi_dispatch
> MEM manually here, set MEM_READONLY_P and MEM_NOTRAP_P on it.
This AArch64 part is OK for trunk.
Thanks,
James
> --- gcc/config/aarch64/aarch64.md.jj 2019-01-19 09:39:18.847831222 +0100
> +++ gcc/config/aarch64/aarch64.md 2019-02-21 15:25:27.874532191 +0100
> @@ -622,13 +622,27 @@ (define_expand "casesi"
> operands[0], operands[2], operands[4]));
>
> operands[2] = force_reg (DImode, gen_rtx_LABEL_REF (DImode, operands[3]));
> - emit_jump_insn (gen_casesi_dispatch (operands[2], operands[0],
> - operands[3]));
> + operands[2]
> + = gen_rtx_UNSPEC (Pmode, gen_rtvec (2, operands[2], operands[0]),
> + UNSPEC_CASESI);
> + operands[2] = gen_rtx_MEM (DImode, operands[2]);
> + MEM_READONLY_P (operands[2]) = 1;
> + MEM_NOTRAP_P (operands[2]) = 1;
> + emit_jump_insn (gen_casesi_dispatch (operands[2], operands[3]));
> DONE;
> }
> )
>
> -(define_insn "casesi_dispatch"
> +(define_expand "casesi_dispatch"
> + [(parallel
> + [(set (pc) (match_operand:DI 0 ""))
> + (clobber (reg:CC CC_REGNUM))
> + (clobber (match_scratch:DI 2))
> + (clobber (match_scratch:DI 3))
> + (use (label_ref:DI (match_operand 1 "")))])]
> + "")
> +
> +(define_insn "*casesi_dispatch"
> [(parallel
> [(set (pc)
> (mem:DI (unspec [(match_operand:DI 0 "register_operand" "r")
> @@ -637,7 +651,7 @@ (define_insn "casesi_dispatch"
> (clobber (reg:CC CC_REGNUM))
> (clobber (match_scratch:DI 3 "=r"))
> (clobber (match_scratch:DI 4 "=r"))
> - (use (label_ref (match_operand 2 "" "")))])]
> + (use (label_ref:DI (match_operand 2 "" "")))])]
> ""
> "*
> return aarch64_output_casesi (operands);
>
> Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-02-27 16:20 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-23 0:29 [PATCH] Improve arm and aarch64 casesi (PR target/70341) Jakub Jelinek
2019-02-25 10:23 ` Kyrill Tkachov
2019-02-25 10:27 ` Jakub Jelinek
2019-02-25 10:29 ` Kyrill Tkachov
2019-02-27 11:21 ` Jakub Jelinek
2019-02-27 12:03 ` Richard Earnshaw (lists)
2019-02-27 15:03 ` Kyrill Tkachov
2019-02-27 15:24 ` Jakub Jelinek
2019-02-27 17:22 ` James Greenhalgh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).