Re: [MSP430][PATCH 1/2] Consolidate handling of hard-coded MCU data

[Jeff Law <law at redhat dot com>]

On 8/8/19 6:14 AM, Jozef Lawrynowicz wrote:
> This patch improves the handling of MCU data by consolidating multiple
> copies of hard-coded MCU data into a single location, and adds a new function
> to be used as a single entry point for the extraction of MCU data for the
> selected MCU.
> 
> This ensures the data is only extracted once per invocation of the
> driver/compiler, whilst previously, the data for the MCU is extracted each time
> it is needed.
> 
> Some notes:
> - The GNU assembler doesn't do anything with the -mmcu option beyond setting up
>   the CPU ISA, so if the GCC driver passes it the -mcpu option, which it will
>   always do if -mmcu is specified, then it is redundant to also pass it -mmcu.
> - The indenting in some places (e.g. msp430_select_hwmult_lib) looks wrong in
>   the patched file, but to make the diff a lot easier to read I have kept the
>   indenting the same as it was before. I can fix this after the patch is
>   accepted.
FWIW, another way to address the indentation issue is with the diff -b
option which says to ignore whitespace differences.

Regardless, yes, please clean up any indentation issues.


> 
> 
> 0001-MSP430-Devices-1-Consolidate-handling-of-hard-coded-.patch
> 
> From cd131b07e0447d104c99317e7ac37c2420c1bf6e Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> Date: Wed, 31 Jul 2019 22:53:50 +0100
> Subject: [PATCH 1/2] MSP430: Devices [1]: Consolidate handling of hard-coded
>  MCU data
> 
> gcc/ChangeLog:
> 
> 2019-08-XX  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* gcc/config.gcc (msp430*-*-*): Add msp430-devices.o to extra_objs and
> 	extra_gcc_objs.
> 	* gcc/config/msp430/driver-msp430.c: Remove msp430_mcu_data.
> 	(msp430_select_cpu): New spec function.
> 	(msp430_select_hwmult_lib): Use msp430_extract_mcu_data to extract
> 	MCU data.
> 	* gcc/config/msp430/msp430-devices.c: New file.
> 	* gcc/config/msp430/msp430-devices.h: New file.
> 	* gcc/config/msp430/msp430.c: Remove msp430_mcu_data.
> 	(msp430_option_override): Use msp430_extract_mcu_data to extract
> 	MCU data.
> 	(msp430_use_f5_series_hwmult): Likewise.
> 	(use_32bit_hwmult): Likewise.
> 	(msp430_no_hwmult): Likewise.
> 	* gcc/config/msp430/msp430.h (ASM_SPEC): Don't pass -mmcu to the
> 	assembler.
> 	(DRIVER_SELF_SPECS): Call msp430_select_cpu if -mmcu is used without
> 	and -mcpu option.
> 	(EXTRA_SPEC_FUNCTIONS): Add msp430_select_cpu.
> 	* gcc/config/msp430/t-msp430: Add rule to build msp430-devices.o.
> 	Remove hard-coded MCU multilib data.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-08-XX  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* gcc.target/msp430/msp430.exp
> 	(check_effective_target_msp430_430_selected): New.
> 	(check_effective_target_msp430_430x_selected): New.
> 	(check_effective_target_msp430_mlarge_selected): New.
> 	(check_effective_target_msp430_hwmul_not_none): New.
> 	(check_effective_target_msp430_hwmul_not_16bit): New.
> 	(check_effective_target_msp430_hwmul_not_32bit): New.
> 	(check_effective_target_msp430_hwmul_not_f5): New.
> 	(msp430_get_opts): New.
> 	(msp430_device_permutations_runtest): New.
> 	* gcc.target/msp430/devices/README: New file.
> 	* gcc.target/msp430/devices-main.c: New test.
> 	* gcc.target/msp430/devices/hard-cc430f5123.c: Likewise.
> 	* gcc.target/msp430/devices/hard-foo.c: Likewise.
> 	* gcc.target/msp430/devices/hard-msp430afe253.c: Likewise.
> 	* gcc.target/msp430/devices/hard-msp430cg4616.c: Likewise.
> 	* gcc.target/msp430/devices/hard-msp430f4783.c: Likewise.
> 	* gcc.target/msp430/devices/hard-rf430frl154h_rom.c: Likewise.
Presumably we aren't supporting switching the selected mcu via
attributes on a per-function basis?

> +/* Main entry point to load the MCU data for the given -mmcu into
> +   extracted_mcu_data.  hard_msp430_mcu_data (initialized at the bottom of this
> +   file) is searched for the MCU name.
> +   This function only needs to be executed once, but it can be first called
> +   from a number of different locations.  */
> +void
> +msp430_extract_mcu_data (const char * mcu_name)
> +{
> +  static int executed = 0;
> +  int i;
> +  if (mcu_name == NULL || executed == 1)
> +    return;
> +  executed = 1;
> +  /* FIXME: This array is alpha sorted - we could use a binary search.  */
> +  for (i = ARRAY_SIZE (hard_msp430_mcu_data); i--;)
> +    if (strcasecmp (mcu_name, hard_msp430_mcu_data[i].name) == 0)
> +      {
> +	extracted_mcu_data = hard_msp430_mcu_data[i];
> +	break;
> +      }
I guess we only run this once, so the linear search isn't a serious
compile-time issue?

Assuming we don't care about switching the selected mcu within a
compilation unit, OK for the trunk.

jeff