This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]