On 9/25/19 10:24 AM, Jozef Lawrynowicz wrote:
> ping
>
> On Wed, 11 Sep 2019 11:25:58 +0100
> Jozef Lawrynowicz <joze...@mittosystems.com> 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 0x10000)
>> and "upper" region (at or above address 0x10000).
>> 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, &foo.
>> 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 <joze...@mittosystems.com>
> 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 <joze...@mittosystems.com>
>
> * 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.
> (bic<mode>3): Likewise.
> (and<mode>3): Likewise.
> (ior<mode>3): Likewise.
> (xor<mode>3): 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): Likewise.
> (*bitbranch<mode>4): Likewise.
> (*bitbranch<mode>4_z): Remove unnecessary "%x" operand selector.
> * config/msp430/msp430.opt (mcode-region=): Set default to
> MSP430_REGION_LOWER. Improve docstring.
> (mdata-region=): Likewise.
> (muse-lower-region-prefix): New option.
> * config/msp430/t-msp430 (MULTILIB_OPTIONS): Add
> mdata-region=none multilib.
> (MULTILIB_MATCHES): Set mdata-region={upper,either} to match
> mdata-region=none multilib.
> MULTILIB_EXCEPTIONS: Remove.
> MULTILIB_REQUIRED: Define.
> * configure: Regenerate.
> * configure.ac: Define HAVE_AS_GNU_ATTRIBUTE and
> HAVE_AS_MSPABI_ATTRIBUTE if GAS version >= 2.32.51.
> * doc/extend.texi: Clarify comment for {upper,lower,either}
> function attributes.
> Add separate description for "lower" variable attribute.
>
> gcc/testsuite/ChangeLog:
>
> 2019-09-11 Jozef Lawrynowicz <joze...@mittosystems.com>
>
> * gcc.target/msp430/430x-insns.c: New test.
> * gcc.target/msp430/data-attributes-2.c: Remove dg-warning
> directives for conflicts between the "section" and "lower" attributes.
> * gcc.target/msp430/msp430.exp
> (check_effective_target_msp430_region_not_lower): New.
> (check_effective_target_msp430_region_lower): New.
> * gcc.target/msp430/object-attributes-430.c: New test.
> * gcc.target/msp430/object-attributes-default.c: New test.
> * gcc.target/msp430/object-attributes-mlarge-any-region.c: New test.
> * gcc.target/msp430/object-attributes-mlarge.c: New test.
So a note, I didn't look at any of the binutils stuff. Just want to be
clear my ACK only applies to the GCC bits.
OK for the trunk.
jeff
