Re: [PATCH 5/5] single_set takes an insn
On 09/08/14 15:29, David Malcolm wrote: gcc/ChangeLog: * rtl.h (single_set_2): Strengthen first param from const_rtx to const rtx_insn *, and move prototype to above... (single_set): ...this. Convert this from a macro to an inline function, enforcing the requirement that the param is a const rtx_insn *. (find_args_size_adjust): Strengthen param from rtx to rtx_insn *. * config/arm/aarch-common-protos.h (aarch_crypto_can_dual_issue): Strengthen both params from rtx to rtx_insn *. * config/arm/aarch-common.c (aarch_crypto_can_dual_issue): Likewise; introduce locals producer_set, consumer_set, using them in place of producer and consumer when dealing with SET rather than insn. * config/avr/avr.c (avr_out_plus): Add checked cast to rtx_insn * when invoking single_set in region guarded by INSN_P. (avr_out_bitop): Likewise. (_reg_unused_after): Introduce local rtx_sequence * seq in region guarded by GET_CODE check, using methods to strengthen local this_insn from rtx to rtx_insn *, and for clarity. * config/avr/avr.md (define_insn_and_split xload8mode_A): Strengthen local insn from rtx to rtx_insn *. (define_insn_and_split xloadmode_A): Likewise. * config/bfin/bfin.c (trapping_loads_p): Likewise for param insn. (find_load): Likewise for return type. (workaround_speculation): Likewise for both locals named load_insn. * config/cris/cris.c (cris_cc0_user_requires_cmp): Likewise for local cc0_user. * config/cris/cris.md (define_peephole2 ; moversideqi): Likewise for local prev. * config/h8300/h8300-protos.h (notice_update_cc): Likewise for param 2. * config/h8300/h8300.c (notice_update_cc): Likewise. * config/i386/i386.c (ix86_flags_dependent): Likewise for params insn and dep_insn. (exact_store_load_dependency): Likewise for both params. (ix86_macro_fusion_pair_p): Eliminate local named single_set since this now clashes with inline function. Instead, delay calling single_set until the point where its needed, and then assign the result to compare_set and rework the conditional that follows. * config/ia64/ia64.md (define_expand tablejump): Strengthen local last from rtx to rtx_insn *. * config/mips/mips-protos.h (mips_load_store_insns): Likewise for second param. (mips_store_data_bypass_p): Likewise for both params. * config/mips/mips.c (mips_load_store_insns): Likewise for second param. (mips_store_data_bypass_p): Likewise for both params. (mips_orphaned_high_part_p): Likewise for param insn. * config/mn10300/mn10300.c (extract_bundle): Likewise. (mn10300_bundle_liw): Likewise for locals r, insn1, insn2. Introduce local rtx insn2_pat. * config/rl78/rl78.c (move_elim_pass): Likewise for locals insn, ninsn. (rl78_remove_unused_sets): Likewise for locals insn, ninsn. Introduce local rtx set, using it in place of insn for the result of single_set. This appears to fix a bug, since the call to find_regno_note on a SET does nothing. * config/rs6000/rs6000.c (set_to_load_agen): Strengthen both params from rtx to rtx_insn *. (set_to_load_agen): Likewise. * config/s390/s390.c (s390_label_align): Likewise for local prev_insn. Introduce new rtx locals set and src, using them in place of prev_insn for the results of single_set and SET_SRC respectively. (s390_swap_cmp): Strengthen local jump from rtx to rtx_insn *. Introduce new rtx local set using in place of jump for the result of single_set. Use SET_SRC (set) rather than plain XEXP (set, 1). * config/sh/sh.c (noncall_uses_reg): Strengthen param 2from rtx to rtx_insn *. (noncall_uses_reg): Likewise. (reg_unused_after): Introduce local rtx_sequence * seq in region guarded by GET_CODE check, using its methods for clarity, and to enable strengthening local this_insn from rtx to rtx_insn *. * config/sh/sh.md (define_expand mulhisi3): Strengthen local insn from rtx to rtx_insn *. (define_expand umulhisi3): Likewise. (define_expand smulsi3_highpart): Likewise. (define_expand umulsi3_highpart): Likewise. * config/sparc/sparc.c (sparc_do_work_around_errata): Likewise for local after. Replace GET_CODE check with a dyn_cast, introducing new local rtx_sequence * seq, using insn method for typesafety. * dwarf2cfi.c (dwarf2out_frame_debug): Strengthen param insn from rtx to rtx_insn *. Introduce local rtx pat, using it in place of insn once we're dealing with patterns rather than the
[PATCH 5/5] single_set takes an insn
On Tue, 2014-09-09 at 09:53 -0600, Jeff Law wrote:
On 09/08/14 15:29, David Malcolm wrote:
gcc/ChangeLog:
[...]
* config/avr/avr.c (avr_out_plus): Add checked cast to rtx_insn *
when invoking single_set in region guarded by INSN_P.
(avr_out_bitop): Likewise.
(_reg_unused_after): Introduce local rtx_sequence * seq in
region guarded by GET_CODE check, using methods to strengthen
local this_insn from rtx to rtx_insn *, and for clarity.
* config/avr/avr.md (define_insn_and_split xload8mode_A):
Strengthen local insn from rtx to rtx_insn *.
(define_insn_and_split xloadmode_A): Likewise.
[...]
diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c
index 70d5db5..e749793 100644
--- a/gcc/config/avr/avr.c
+++ b/gcc/config/avr/avr.c
@@ -6769,7 +6769,7 @@ avr_out_plus (rtx insn, rtx *xop, int *plen, int
*pcc, bool out_label)
int cc_plus, cc_minus, cc_dummy;
int len_plus, len_minus;
rtx op[4];
- rtx xpattern = INSN_P (insn) ? single_set (insn) : insn;
+ rtx xpattern = INSN_P (insn) ? single_set (as_a rtx_insn * (insn)) :
insn;
Whee, presumably a new item for the TODO list :-) insn here could be
strengthened to an rtx_insn.
[dropping DJ from CC as this doesn't relate to rl78]
[adding Denis to CC due to avr-related discussion]
The reason for the as_a rtx_insn * here is that avr_out_plus and
avr_out_bitop are mostly called with insns [1], but are also called with
SET patterns [2], which is what the changed line above is meant to
handle.
Perhaps a fix could be to split each into two functions, perhaps:
static avr_out_plus_set
extern avr_out_plus_insn
with the latter calling the former (the former is only used from
avr_out_round).
I did start looking at this, but the logic in avr_out_plus makes it
non-trivial:
6766 const char*
6767 avr_out_plus (rtx insn, rtx *xop, int *plen, int *pcc, bool out_label)
6768 {
6769int cc_plus, cc_minus, cc_dummy;
6770int len_plus, len_minus;
6771rtx op[4];
6772rtx xpattern = INSN_P (insn) ? single_set (as_a rtx_insn * (insn))
: insn;
6773rtx xdest = SET_DEST (xpattern);
6774enum machine_mode mode = GET_MODE (xdest);
6775enum machine_mode imode = int_mode_for_mode (mode);
6776int n_bytes = GET_MODE_SIZE (mode);
6777enum rtx_code code_sat = GET_CODE (SET_SRC (xpattern));
6778enum rtx_code code
6779 = (PLUS == code_sat || SS_PLUS == code_sat || US_PLUS == code_sat
6780 ? PLUS : MINUS);
6781
6782if (!pcc)
6783 pcc = cc_dummy;
6784
6785/* PLUS and MINUS don't saturate: Use modular wrap-around. */
6786
6787if (PLUS == code_sat || MINUS == code_sat)
6788 code_sat = UNKNOWN;
6789
6790if (n_bytes = 4 REG_P (xop[2]))
6791 {
6792avr_out_plus_1 (xop, plen, code, pcc, code_sat, 0, out_label);
6793return ;
6794 }
6795
6796if (8 == n_bytes)
6797 {
6798op[0] = gen_rtx_REG (DImode, ACC_A);
6799op[1] = gen_rtx_REG (DImode, ACC_A);
6800op[2] = avr_to_int_mode (xop[0]);
6801 }
6802else
6803 {
6804if (!REG_P (xop[2])
6805 !CONST_INT_P (xop[2])
6806 !CONST_FIXED_P (xop[2]))
6807 {
6808return avr_out_plus_symbol (xop, code, plen, pcc);
6809 }
6810
6811op[0] = avr_to_int_mode (xop[0]);
6812op[1] = avr_to_int_mode (xop[1]);
6813op[2] = avr_to_int_mode (xop[2]);
6814 }
6815
6816/* Saturations and 64-bit operations don't have a clobber operand.
6817 For the other cases, the caller will provide a proper XOP[3]. */
6818
6819xpattern = INSN_P (insn) ? PATTERN (insn) : insn;
6820op[3] = PARALLEL == GET_CODE (xpattern) ? xop[3] : NULL_RTX;
Note how in line 6773 we use SET_DEST to get xdest, and then proceed
to use xdest, so presumably insn was a SET, or an insn containing a SET
(perhaps within a PARALLEL) at line 6772.
But in line 6819 we go back to insn and either treat it as a pattern,
or re-extract the pattern, and then at line 6820 consider that it could
be a PARALLEL.
Given that lines 6819 onwards do a bunch of additional stuff, it seemed
easiest for now to simply handle both the pattern and insn cases in the
same function, rather than try to split them. Of course, this would
need to be split out if we go ahead with the insns are no longer a
subclass of rtx idea.
I won't call them all out since we can grep for as_a rtx_insn to find
these in the future.
OK for the trunk,
Thanks; r215089, fwiw.
Dave
[1] from avr_notice_update_cc, from avr_adjust_insn_length and from
clauses in the .md file
[2] once in each case, from avr_out_round, for both _plus and _bitop.
Re: [PATCH 5/5] single_set takes an insn
On 09/09/14 11:34, David Malcolm wrote: [dropping DJ from CC as this doesn't relate to rl78] [adding Denis to CC due to avr-related discussion] The reason for the as_a rtx_insn * here is that avr_out_plus and avr_out_bitop are mostly called with insns [1], but are also called with SET patterns [2], which is what the changed line above is meant to handle. Egad. Well, this is precisely the kind of thing we want to put on that list of things to clean up. The as-a highlights it reasonably well :-) It's lower priority than a lot of the other cleanups and with the as-a as a marker, I'm comfortable leaving it as-is for now. jeff
[PATCH 5/5] single_set takes an insn
gcc/ChangeLog: * rtl.h (single_set_2): Strengthen first param from const_rtx to const rtx_insn *, and move prototype to above... (single_set): ...this. Convert this from a macro to an inline function, enforcing the requirement that the param is a const rtx_insn *. (find_args_size_adjust): Strengthen param from rtx to rtx_insn *. * config/arm/aarch-common-protos.h (aarch_crypto_can_dual_issue): Strengthen both params from rtx to rtx_insn *. * config/arm/aarch-common.c (aarch_crypto_can_dual_issue): Likewise; introduce locals producer_set, consumer_set, using them in place of producer and consumer when dealing with SET rather than insn. * config/avr/avr.c (avr_out_plus): Add checked cast to rtx_insn * when invoking single_set in region guarded by INSN_P. (avr_out_bitop): Likewise. (_reg_unused_after): Introduce local rtx_sequence * seq in region guarded by GET_CODE check, using methods to strengthen local this_insn from rtx to rtx_insn *, and for clarity. * config/avr/avr.md (define_insn_and_split xload8mode_A): Strengthen local insn from rtx to rtx_insn *. (define_insn_and_split xloadmode_A): Likewise. * config/bfin/bfin.c (trapping_loads_p): Likewise for param insn. (find_load): Likewise for return type. (workaround_speculation): Likewise for both locals named load_insn. * config/cris/cris.c (cris_cc0_user_requires_cmp): Likewise for local cc0_user. * config/cris/cris.md (define_peephole2 ; moversideqi): Likewise for local prev. * config/h8300/h8300-protos.h (notice_update_cc): Likewise for param 2. * config/h8300/h8300.c (notice_update_cc): Likewise. * config/i386/i386.c (ix86_flags_dependent): Likewise for params insn and dep_insn. (exact_store_load_dependency): Likewise for both params. (ix86_macro_fusion_pair_p): Eliminate local named single_set since this now clashes with inline function. Instead, delay calling single_set until the point where its needed, and then assign the result to compare_set and rework the conditional that follows. * config/ia64/ia64.md (define_expand tablejump): Strengthen local last from rtx to rtx_insn *. * config/mips/mips-protos.h (mips_load_store_insns): Likewise for second param. (mips_store_data_bypass_p): Likewise for both params. * config/mips/mips.c (mips_load_store_insns): Likewise for second param. (mips_store_data_bypass_p): Likewise for both params. (mips_orphaned_high_part_p): Likewise for param insn. * config/mn10300/mn10300.c (extract_bundle): Likewise. (mn10300_bundle_liw): Likewise for locals r, insn1, insn2. Introduce local rtx insn2_pat. * config/rl78/rl78.c (move_elim_pass): Likewise for locals insn, ninsn. (rl78_remove_unused_sets): Likewise for locals insn, ninsn. Introduce local rtx set, using it in place of insn for the result of single_set. This appears to fix a bug, since the call to find_regno_note on a SET does nothing. * config/rs6000/rs6000.c (set_to_load_agen): Strengthen both params from rtx to rtx_insn *. (set_to_load_agen): Likewise. * config/s390/s390.c (s390_label_align): Likewise for local prev_insn. Introduce new rtx locals set and src, using them in place of prev_insn for the results of single_set and SET_SRC respectively. (s390_swap_cmp): Strengthen local jump from rtx to rtx_insn *. Introduce new rtx local set using in place of jump for the result of single_set. Use SET_SRC (set) rather than plain XEXP (set, 1). * config/sh/sh.c (noncall_uses_reg): Strengthen param 2from rtx to rtx_insn *. (noncall_uses_reg): Likewise. (reg_unused_after): Introduce local rtx_sequence * seq in region guarded by GET_CODE check, using its methods for clarity, and to enable strengthening local this_insn from rtx to rtx_insn *. * config/sh/sh.md (define_expand mulhisi3): Strengthen local insn from rtx to rtx_insn *. (define_expand umulhisi3): Likewise. (define_expand smulsi3_highpart): Likewise. (define_expand umulsi3_highpart): Likewise. * config/sparc/sparc.c (sparc_do_work_around_errata): Likewise for local after. Replace GET_CODE check with a dyn_cast, introducing new local rtx_sequence * seq, using insn method for typesafety. * dwarf2cfi.c (dwarf2out_frame_debug): Strengthen param insn from rtx to rtx_insn *. Introduce local rtx pat, using it in place of insn once we're dealing with patterns rather than the input insn. (scan_insn_after):
