Re: [PATCH v2] xtensa: Make full transition to LRA

[Jeff Law <jeffreyalaw@gmail.com>]

On 5/8/23 08:44, Takayuki 'January June' Suwa via Gcc-patches wrote:
> On 2023/05/08 22:43, Richard Biener wrote:
> [snip]
>>> -mlra
>>
>> If they were in any released compiler options should be kept
>> (doing nothing) for backward compatibility.  Use for example
>>
>> mlra
>> Target WarnRemoved
>> Removed in GCC 14.  This switch has no effect.
>>
>> or
>>
>> mlra
>> Target Ignore
>> Does nothing.  Preserved for backward compatibility.
>>
>> which doesn't inform the user (I think that's the better choice here).
>>
>>> -Target Mask(LRA)
> Thank you for your helpful advice.
> 
> =====
> gcc/ChangeLog:
> 
> 	* config/xtensa/constraints.md (R, T, U):
> 	Change define_constraint to define_memory_constraint.
> 	* config/xtensa/xtensa.cc
> 	(xtensa_lra_p, TARGET_LRA_P): Remove.
> 	(xtensa_emit_move_sequence): Remove "if (reload_in_progress)"
> 	clause as it can no longer be true.
> 	(xtensa_output_integer_literal_parts): Consider 16-bit wide
> 	constants.
> 	(xtensa_legitimate_constant_p): Add short-circuit path for
> 	integer load instructions.
> 	* config/xtensa/xtensa.md (movsf): Use can_create_pseudo_p()
> 	rather reload_in_progress and reload_completed.
> 	* config/xtensa/xtensa.opt (mlra): Change to no effect.
> ---
>   gcc/config/xtensa/constraints.md | 26 ++++++++------------------
>   gcc/config/xtensa/xtensa.cc      | 26 +++++---------------------
>   gcc/config/xtensa/xtensa.md      |  2 +-
>   gcc/config/xtensa/xtensa.opt     |  4 ++--
>   4 files changed, 16 insertions(+), 42 deletions(-)
> 
> diff --git a/gcc/config/xtensa/constraints.md b/gcc/config/xtensa/constraints.md
> index 53e4d0d8dd1..9b31e162941 100644
> --- a/gcc/config/xtensa/constraints.md
> +++ b/gcc/config/xtensa/constraints.md
> @@ -123,29 +123,19 @@
>         (and (match_code "const_int")
>   	   (match_test "! xtensa_split1_finished_p ()"))))
>   
> -;; Memory constraints.  Do not use define_memory_constraint here.  Doing so
> -;; causes reload to force some constants into the constant pool, but since
> -;; the Xtensa constant pool can only be accessed with L32R instructions, it
> -;; is always better to just copy a constant into a register.  Instead, use
> -;; regular constraints but add a check to allow pseudos during reload.
> +;; Memory constraints.
>   
> -(define_constraint "R"
> +(define_memory_constraint "R"
>    "Memory that can be accessed with a 4-bit unsigned offset from a register."
> - (ior (and (match_code "mem")
> -	   (match_test "smalloffset_mem_p (op)"))
> -      (and (match_code "reg")
> -	   (match_test "reload_in_progress
> -			&& REGNO (op) >= FIRST_PSEUDO_REGISTER"))))
> + (and (match_code "mem")
> +      (match_test "smalloffset_mem_p (op)")))
>   
> -(define_constraint "T"
> +(define_memory_constraint "T"
>    "Memory in a literal pool (addressable with an L32R instruction)."
>    (and (match_code "mem")
>         (match_test "!TARGET_CONST16 && constantpool_mem_p (op)")))
>   
> -(define_constraint "U"
> +(define_memory_constraint "U"
>    "Memory that is not in a literal pool."
> - (ior (and (match_code "mem")
> -	   (match_test "! constantpool_mem_p (op)"))
> -      (and (match_code "reg")
> -	   (match_test "reload_in_progress
> -			&& REGNO (op) >= FIRST_PSEUDO_REGISTER"))))
> + (and (match_code "mem")
> +      (match_test "! constantpool_mem_p (op)")))
Given the old comment, you might want to use 
define_special_memory_constraint rather than define_memory_constraint.

It might be worth doing some before/after comparisons to see if there's 
any noticeable impact.

jeff