Hi Eric.
> The niagara7 pipeline description models the V3 pipe using a bypass
> with latency 3, from-to any instruction executing in the V3 pipe. The
> instructions are identified by mean of a new instruction attribute
> "v3pipe", that has been added to the proper define_insns in sparc.md.
>
> However, I am not sure how well this will scale ni the future, as in
> future cpu models the subset of instruction executing in the V3 pipe
> will be different than in the M7. So we need a way to define that
> instruction class that is processor-specific: using v3pipe, v3pipe_m8,
> v3pipe_m9, etc seems a bit messy to me. Any idea?
I guess it depends on whether the set of instructions executing in the V3
pipe
is (or will become) kind of arbitrary or not. The usual way to support a
new
scheduling model is to define new (sub-)types of instructions and assign
the
appropriate (sub-)types to the scheduling units but, of course, if affected
instructions are selected randomly, the model falls apart.
Yes, it is arbitrary. My first attempt was indeed to follow the
instruction type approach, but then I couldn't even find proper names
for the categories.
I think I will change v3pipe to v3pipe_m7 in this patch, to make it more
explicit that the insn<->v3pipe association is processor-dependant.
Wdyt?
> @@ -1583,10 +1626,29 @@ sparc_option_override (void)
>
> || sparc_cpu == PROCESSOR_NIAGARA
> || sparc_cpu == PROCESSOR_NIAGARA2
> || sparc_cpu == PROCESSOR_NIAGARA3
>
> - || sparc_cpu == PROCESSOR_NIAGARA4)
> - ? 64 : 32),
> + || sparc_cpu == PROCESSOR_NIAGARA4
> + || sparc_cpu == PROCESSOR_NIAGARA7)
> + ? 32 : 64),
> + global_options.x_param_values,
> + global_options_set.x_param_values);
> + maybe_set_param_value (PARAM_L1_CACHE_SIZE,
> + ((sparc_cpu == PROCESSOR_ULTRASPARC
> + || sparc_cpu == PROCESSOR_ULTRASPARC3
> + || sparc_cpu == PROCESSOR_NIAGARA
> + || sparc_cpu == PROCESSOR_NIAGARA2
> + || sparc_cpu == PROCESSOR_NIAGARA3
> + || sparc_cpu == PROCESSOR_NIAGARA4
> + || sparc_cpu == PROCESSOR_NIAGARA7)
> + ? 16 : 64),
> global_options.x_param_values,
> global_options_set.x_param_values);
> + maybe_set_param_value (PARAM_L2_CACHE_SIZE,
> + (sparc_cpu == PROCESSOR_NIAGARA4
> + ? 128 : (sparc_cpu == PROCESSOR_NIAGARA7
> + ? 256 : 512)),
> + global_options.x_param_values,
> + global_options_set.x_param_values);
> +
Please add a blank line between the statements. Why swapping 32 and 64 for
PARAM_L1_CACHE_LINE_SIZE? If the 32 default is universally OK, then let's
just remove the statement.
Yes, I agree, I will just remove the statement. (When I wrote that I was
having in mind future models of the CPU that will actually feature L1D$
64 bit lines.)
> /* Disable save slot sharing for call-clobbered registers by default.
> The IRA sharing algorithm works on single registers only and this
> @@ -9178,7 +9240,8 @@ sparc32_initialize_trampoline (rtx m_tramp, rtx
> fnaddr, rtx cxt) && sparc_cpu != PROCESSOR_NIAGARA
> && sparc_cpu != PROCESSOR_NIAGARA2
> && sparc_cpu != PROCESSOR_NIAGARA3
> - && sparc_cpu != PROCESSOR_NIAGARA4)
> + && sparc_cpu != PROCESSOR_NIAGARA4
> + && sparc_cpu != PROCESSOR_NIAGARA7) /* XXX */
> emit_insn (gen_flushsi (validize_mem (adjust_address (m_tramp,
SImode,
> 8))));
What does the "XXX" mean?
Oops, these are unintended leftovers, sorry.
> diff --git a/gcc/config/sparc/sparc.h b/gcc/config/sparc/sparc.h
> index ebfe87d..d91496a 100644
> --- a/gcc/config/sparc/sparc.h
> +++ b/gcc/config/sparc/sparc.h
> @@ -142,6 +142,7 @@ extern enum cmodel sparc_cmodel;
> #define TARGET_CPU_niagara2 14
> #define TARGET_CPU_niagara3 15
> #define TARGET_CPU_niagara4 16
> +#define TARGET_CPU_niagara7 19
Any contribution to plug the hole is of course welcome. :-)
:) Maybe some day.
> +(define_insn "<vis3_addsub_ss_patname>v8qi3"
> + [(set (match_operand:V8QI 0 "register_operand" "=e")
> + (vis3_addsub_ss:V8QI (match_operand:V8QI 1 "register_operand"
"e")
> + (match_operand:V8QI 2 "register_operand"
> "e")))] + "TARGET_VIS4"
> + "<vis3_addsub_ss_insn>8\t%1, %2, %0"
> + [(set_attr "type" "fga")])
If the mix of VIS4 and vis3_ is correct, then this requires a comment.
Yes, it is intended. I will add a little note.
I will also fix all the other points you raised.
Thanks!
