Re: [ping][PATCH][MSP430] Don't generate 430X insns when handling data in the lower memory region

2019-10-04 Thread Jeff Law
On 9/25/19 10:24 AM, Jozef Lawrynowicz wrote:
> ping
> 
> On Wed, 11 Sep 2019 11:25:58 +0100
> Jozef Lawrynowicz  wrote:
> 
>> The MSP430 target has a "430X" extension which increases the directly
>> addressable memory range from 64KB (16-bit) to 1MB (20-bit).
>> This 1MB memory range is split into a "lower" region (below address 0x1)
>> and "upper" region (at or above address 0x1).
>> When data in the upper region is addressed, 430 instructions cannot be used, 
>> as
>> their 16-bit capability will be exceeded; 430X instructions must be used
>> instead. Most 430X instructions require an additional word of op-code, and 
>> also
>> require more cycles to execute compared to their 430 equivalent.
>>
>> Currently, when the large memory model is specified (-mlarge), 430X 
>> instructions
>> will always be used when addressing a symbol_ref using the absolute 
>> addressing
>> mode e.g. MOVX #1, 
>> The attached patch modifies code generation so that 430X instructions will 
>> only
>> be used when the symbol being addressed will not be placed in the lower 
>> memory
>> region. This is determined by checking if -mdata-region=lower (the new 
>> default)
>> is passed, or if the "lower" attribute is set on the variable.
>>
>> Since code will be generated to assume all variables are in the lower memory
>> region with -mdata-region=lower, object files built with this option cannot
>> be linked with objects files built with other -mdata-region= values.
>> To facilitate the checking of this, a patch for binutils (to be submitted
>> after this is accepted) is also attached.
>>
>> The compiler will now generate assembler directives indicating the ISA, 
>> memory
>> model and data region the source file was compiled with. The assembler will
>> check these directives match the options it has been invoked with, and then
>> add the attribute to the object file.
>>
>> Successfully regtested for msp430-elf in the small and large memory model, 
>> and
>> with -mdata-region=upper. Testing with -mdata-region=upper should expose any
>> cases where a 430X instruction is not used where it is required, since all 
>> data
>> is forced into the upper region so a lack of 430X insn would cause a 
>> relocation
>> overflow. In fact the attached patch fixes some relocation overflows by 
>> adding
>> missing "%X" operand selectors to some insns. One relocation overflow remains
>> (pr65077.c), but that is a separate binutils issue.
>>
>> Ok for trunk?
> 
> 
> 0001-MSP430-Only-generate-430X-instructions-with-mlarge-i.patch
> 
> From 91371f9a2721e1459429ff7ebdb258b2ef063b04 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz 
> Date: Wed, 14 Aug 2019 13:25:03 +0100
> Subject: [PATCH] MSP430: Only generate 430X instructions with -mlarge if data
>  will be in the upper region
> 
> gcc/ChangeLog:
> 
> 2019-09-11  Jozef Lawrynowicz  
> 
>   * config.in: Regenerate.
>   * config/msp430/constraints.md: Fix docstring for "Ys" constraint.
>   Add new "Yx" constraint.
>   * config/msp430/driver-msp430.c (msp430_propagate_region_opt): New spec
>   function.
>   * config/msp430/msp430-protos.h (msp430_op_not_in_high_mem): New
>   prototype.
>   * config/msp430/msp430.c (msp430_option_override): Allow the lower
>   code/data region to be selected in the small memory model.
>   (msp430_section_attr): Don't warn if the "section" and "lower"
>   attributes are used together.
>   (msp430_handle_generic_attribute): Likewise.
>   (msp430_var_in_low_mem): New function.
>   (TARGET_ENCODE_SECTION_INFO): Define.
>   (msp430_encode_section_info): New function.
>   (gen_prefix): Return early in the small memory model.
>   Require TARGET_USE_LOWER_REGION_PREFIX to be set before adding the
>   ".lower" prefix if -m{code,data}-region=lower have been passed.
>   (msp430_output_aligned_decl_common): Emit common symbols when
>   -mdata-region=lower is passed unless TARGET_USE_LOWER_REGION_PREFIX is
>   set. 
>   (TARGET_ASM_FILE_END): Define.
>   (msp430_file_end): New function.
>   (msp430_do_not_relax_short_jumps): Allow relaxation when
>   function will be in the lower region.
>   (msp430_op_not_in_high_mem): New function.
>   (msp430_print_operand): Check "msp430_op_not_in_high_mem" for
>   the 'X' operand selector. 
>   Clarify comment for 'x' operand selector.
>   * config/msp430/msp430.h (LINK_SPEC): Propagate
>   -m{code,data}-region to the linker via spec function
>   msp430_propagate_region_opt.
>   (msp430_propagate_region_opt): New prototype.
>   (EXTRA_SPEC_FUNCTIONS): Add msp430_propagate_region_opt.
>   (SYMBOL_FLAG_LOW_MEM): Define.
>   * config/msp430/msp430.md (addsipsi3): Add missing "%X" operand
>   selector.
>   (zero_extendqihi2): Fix operand number used by "%X" selector.
>   (zero_extendqisi2): Likewise.
>   (zero_extendhisi2): Likewise.
>   (movqi): Use "Yx" constraint in place of 

[ping][PATCH][MSP430] Don't generate 430X insns when handling data in the lower memory region

2019-09-25 Thread Jozef Lawrynowicz
ping

On Wed, 11 Sep 2019 11:25:58 +0100
Jozef Lawrynowicz  wrote:

> The MSP430 target has a "430X" extension which increases the directly
> addressable memory range from 64KB (16-bit) to 1MB (20-bit).
> This 1MB memory range is split into a "lower" region (below address 0x1)
> and "upper" region (at or above address 0x1).
> When data in the upper region is addressed, 430 instructions cannot be used, 
> as
> their 16-bit capability will be exceeded; 430X instructions must be used
> instead. Most 430X instructions require an additional word of op-code, and 
> also
> require more cycles to execute compared to their 430 equivalent.
> 
> Currently, when the large memory model is specified (-mlarge), 430X 
> instructions
> will always be used when addressing a symbol_ref using the absolute addressing
> mode e.g. MOVX #1, 
> The attached patch modifies code generation so that 430X instructions will 
> only
> be used when the symbol being addressed will not be placed in the lower memory
> region. This is determined by checking if -mdata-region=lower (the new 
> default)
> is passed, or if the "lower" attribute is set on the variable.
> 
> Since code will be generated to assume all variables are in the lower memory
> region with -mdata-region=lower, object files built with this option cannot
> be linked with objects files built with other -mdata-region= values.
> To facilitate the checking of this, a patch for binutils (to be submitted
> after this is accepted) is also attached.
> 
> The compiler will now generate assembler directives indicating the ISA, memory
> model and data region the source file was compiled with. The assembler will
> check these directives match the options it has been invoked with, and then
> add the attribute to the object file.
> 
> Successfully regtested for msp430-elf in the small and large memory model, and
> with -mdata-region=upper. Testing with -mdata-region=upper should expose any
> cases where a 430X instruction is not used where it is required, since all 
> data
> is forced into the upper region so a lack of 430X insn would cause a 
> relocation
> overflow. In fact the attached patch fixes some relocation overflows by adding
> missing "%X" operand selectors to some insns. One relocation overflow remains
> (pr65077.c), but that is a separate binutils issue.
> 
> Ok for trunk?

>From 91371f9a2721e1459429ff7ebdb258b2ef063b04 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz 
Date: Wed, 14 Aug 2019 13:25:03 +0100
Subject: [PATCH] MSP430: Only generate 430X instructions with -mlarge if data
 will be in the upper region

gcc/ChangeLog:

2019-09-11  Jozef Lawrynowicz  

	* config.in: Regenerate.
	* config/msp430/constraints.md: Fix docstring for "Ys" constraint.
	Add new "Yx" constraint.
	* config/msp430/driver-msp430.c (msp430_propagate_region_opt): New spec
	function.
	* config/msp430/msp430-protos.h (msp430_op_not_in_high_mem): New
	prototype.
	* config/msp430/msp430.c (msp430_option_override): Allow the lower
	code/data region to be selected in the small memory model.
	(msp430_section_attr): Don't warn if the "section" and "lower"
	attributes are used together.
	(msp430_handle_generic_attribute): Likewise.
	(msp430_var_in_low_mem): New function.
	(TARGET_ENCODE_SECTION_INFO): Define.
	(msp430_encode_section_info): New function.
	(gen_prefix): Return early in the small memory model.
	Require TARGET_USE_LOWER_REGION_PREFIX to be set before adding the
	".lower" prefix if -m{code,data}-region=lower have been passed.
	(msp430_output_aligned_decl_common): Emit common symbols when
	-mdata-region=lower is passed unless TARGET_USE_LOWER_REGION_PREFIX is
	set. 
	(TARGET_ASM_FILE_END): Define.
	(msp430_file_end): New function.
	(msp430_do_not_relax_short_jumps): Allow relaxation when
	function will be in the lower region.
	(msp430_op_not_in_high_mem): New function.
	(msp430_print_operand): Check "msp430_op_not_in_high_mem" for
	the 'X' operand selector. 
	Clarify comment for 'x' operand selector.
	* config/msp430/msp430.h (LINK_SPEC): Propagate
	-m{code,data}-region to the linker via spec function
	msp430_propagate_region_opt.
	(msp430_propagate_region_opt): New prototype.
	(EXTRA_SPEC_FUNCTIONS): Add msp430_propagate_region_opt.
	(SYMBOL_FLAG_LOW_MEM): Define.
	* config/msp430/msp430.md (addsipsi3): Add missing "%X" operand
	selector.
	(zero_extendqihi2): Fix operand number used by "%X" selector.
	(zero_extendqisi2): Likewise.
	(zero_extendhisi2): Likewise.
	(movqi): Use "Yx" constraint in place of "%X" operand selector.
	(movhi): Likewise.
	(addqi3): Likewise.
	(addhi3): Likewise.
	(addsi3): Likewise.
	(addhi3_cy): Likewise.
	(addchi4_cy): Likewise.
	(subqi3): Likewise.
	(subhi3): Likewise.
	(subsi3): Likewise.
	(bic3): Likewise.
	(and3): Likewise.
	(ior3): Likewise.
	(xor3): Likewise.
	(slli_1): Add missing "%X" operand selector.
	(slll_1): Likewise.
	(slll_2): Likewise.
	(srai_1): Likewise.
	(sral_1): Likewise.
	(sral_2): Likewise.
	(srli_1): Likewise.
	(srll_1): Likewise.
	

[PATCH][MSP430] Don't generate 430X insns when handling data in the lower memory region

2019-09-11 Thread Jozef Lawrynowicz
The MSP430 target has a "430X" extension which increases the directly
addressable memory range from 64KB (16-bit) to 1MB (20-bit).
This 1MB memory range is split into a "lower" region (below address 0x1)
and "upper" region (at or above address 0x1).
When data in the upper region is addressed, 430 instructions cannot be used, as
their 16-bit capability will be exceeded; 430X instructions must be used
instead. Most 430X instructions require an additional word of op-code, and also
require more cycles to execute compared to their 430 equivalent.

Currently, when the large memory model is specified (-mlarge), 430X instructions
will always be used when addressing a symbol_ref using the absolute addressing
mode e.g. MOVX #1, 
The attached patch modifies code generation so that 430X instructions will only
be used when the symbol being addressed will not be placed in the lower memory
region. This is determined by checking if -mdata-region=lower (the new default)
is passed, or if the "lower" attribute is set on the variable.

Since code will be generated to assume all variables are in the lower memory
region with -mdata-region=lower, object files built with this option cannot
be linked with objects files built with other -mdata-region= values.
To facilitate the checking of this, a patch for binutils (to be submitted
after this is accepted) is also attached.

The compiler will now generate assembler directives indicating the ISA, memory
model and data region the source file was compiled with. The assembler will
check these directives match the options it has been invoked with, and then
add the attribute to the object file.

Successfully regtested for msp430-elf in the small and large memory model, and
with -mdata-region=upper. Testing with -mdata-region=upper should expose any
cases where a 430X instruction is not used where it is required, since all data
is forced into the upper region so a lack of 430X insn would cause a relocation
overflow. In fact the attached patch fixes some relocation overflows by adding
missing "%X" operand selectors to some insns. One relocation overflow remains
(pr65077.c), but that is a separate binutils issue.

Ok for trunk?
>From 91371f9a2721e1459429ff7ebdb258b2ef063b04 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz 
Date: Wed, 14 Aug 2019 13:25:03 +0100
Subject: [PATCH] MSP430: Only generate 430X instructions with -mlarge if data
 will be in the upper region

gcc/ChangeLog:

2019-09-11  Jozef Lawrynowicz  

	* config.in: Regenerate.
	* config/msp430/constraints.md: Fix docstring for "Ys" constraint.
	Add new "Yx" constraint.
	* config/msp430/driver-msp430.c (msp430_propagate_region_opt): New spec
	function.
	* config/msp430/msp430-protos.h (msp430_op_not_in_high_mem): New
	prototype.
	* config/msp430/msp430.c (msp430_option_override): Allow the lower
	code/data region to be selected in the small memory model.
	(msp430_section_attr): Don't warn if the "section" and "lower"
	attributes are used together.
	(msp430_handle_generic_attribute): Likewise.
	(msp430_var_in_low_mem): New function.
	(TARGET_ENCODE_SECTION_INFO): Define.
	(msp430_encode_section_info): New function.
	(gen_prefix): Return early in the small memory model.
	Require TARGET_USE_LOWER_REGION_PREFIX to be set before adding the
	".lower" prefix if -m{code,data}-region=lower have been passed.
	(msp430_output_aligned_decl_common): Emit common symbols when
	-mdata-region=lower is passed unless TARGET_USE_LOWER_REGION_PREFIX is
	set. 
	(TARGET_ASM_FILE_END): Define.
	(msp430_file_end): New function.
	(msp430_do_not_relax_short_jumps): Allow relaxation when
	function will be in the lower region.
	(msp430_op_not_in_high_mem): New function.
	(msp430_print_operand): Check "msp430_op_not_in_high_mem" for
	the 'X' operand selector. 
	Clarify comment for 'x' operand selector.
	* config/msp430/msp430.h (LINK_SPEC): Propagate
	-m{code,data}-region to the linker via spec function
	msp430_propagate_region_opt.
	(msp430_propagate_region_opt): New prototype.
	(EXTRA_SPEC_FUNCTIONS): Add msp430_propagate_region_opt.
	(SYMBOL_FLAG_LOW_MEM): Define.
	* config/msp430/msp430.md (addsipsi3): Add missing "%X" operand
	selector.
	(zero_extendqihi2): Fix operand number used by "%X" selector.
	(zero_extendqisi2): Likewise.
	(zero_extendhisi2): Likewise.
	(movqi): Use "Yx" constraint in place of "%X" operand selector.
	(movhi): Likewise.
	(addqi3): Likewise.
	(addhi3): Likewise.
	(addsi3): Likewise.
	(addhi3_cy): Likewise.
	(addchi4_cy): Likewise.
	(subqi3): Likewise.
	(subhi3): Likewise.
	(subsi3): Likewise.
	(bic3): Likewise.
	(and3): Likewise.
	(ior3): Likewise.
	(xor3): Likewise.
	(slli_1): Add missing "%X" operand selector.
	(slll_1): Likewise.
	(slll_2): Likewise.
	(srai_1): Likewise.
	(sral_1): Likewise.
	(sral_2): Likewise.
	(srli_1): Likewise.
	(srll_1): Likewise.
	(cbranchqi4_real): Use "Yx" constraint in place of "%X" operand
	selector.
	(cbranchhi4_real): Likewise.
	(cbranchqi4_reversed): Likewise.
	(cbranchhi4_reversed):