This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch,AVR]: Fix PR29560 (map 16-bit shift to 8-bit)
Richard Henderson wrote:
>> +;; "*ashluqihiqi3.mem"
>> +;; "*ashlsqihiqi3.mem"
>> +(define_insn_and_split "*ashl<extend_prefix>qihiqi3.mem"
>> + [(set (match_operand:QI 0 "memory_operand" "=m")
>> + (subreg:QI (ashift:HI (any_extend:HI (match_operand:QI 1 "register_operand" "r"))
>> + (match_operand:QI 2 "register_operand" "r"))
>> + 0))]
>> + "!reload_completed"
>> + { gcc_unreachable(); }
>
> Surely this isn't necessary. Why would you ever be matching a memory output?
>
>> +(define_insn_and_split "*ashlhiqi3"
>> + [(set (match_operand:QI 0 "nonimmediate_operand" "=r")
>> + (subreg:QI (ashift:HI (match_operand:HI 1 "register_operand" "0")
>> + (match_operand:QI 2 "register_operand" "r")) 0))]
>> + "!reload_completed"
>> + { gcc_unreachable(); }
>
> Likewise.
>
> But the first pattern and the peep2 look good.
>
It's that what combine comes up with, and combine is not smart enough
to find a split point between the mem and the subreg. I don't know
enough of combine, maybe it's because can_create_pseudo_p is false
during combine, combine has no spare reg. A combine-split won't
help as it needs a pseudo/spare reg.
As consequence there is better code if memory operand is allowed
which is a typical use-case, e.g. setting some bits in a SFR.
Johann
- References:
- [Patch,AVR]: Fix PR29560 (map 16-bit shift to 8-bit)
- Re: [Patch,AVR]: Fix PR29560 (map 16-bit shift to 8-bit)
- Re: [Patch,AVR]: Fix PR29560 (map 16-bit shift to 8-bit)
- Re: [Patch,AVR]: Fix PR29560 (map 16-bit shift to 8-bit)
- Re: [Patch,AVR]: Fix PR29560 (map 16-bit shift to 8-bit)
- Re: [Patch,AVR]: Fix PR29560 (map 16-bit shift to 8-bit)
- Re: [Patch,AVR]: Fix PR29560 (map 16-bit shift to 8-bit)
- Re: [Patch,AVR]: Fix PR29560 (map 16-bit shift to 8-bit)
- Re: [Patch,AVR]: Fix PR29560 (map 16-bit shift to 8-bit)
- Re: [Patch,AVR]: Fix PR29560 (map 16-bit shift to 8-bit)