Mikael Pettersson <mikpeli...@gmail.com> writes:

> I have a toy backend based on the moxie backend as a template.  During its
> development I found some oddities in the moxie backend that may be bugs.
>
> 1. The REGNO_OK_FOR_INDEX_P(NUM) macro in moxie.h is:
>
> #define REGNO_OK_FOR_INDEX_P(NUM) MOXIE_FP
>
> Since MOXIE_FP is 0, this returns false for every register.  Should the body
> be a literal 0, or some comparison between NUM and MOXIE_FP?

Great catch!  You are probably right.

> 2. I see no actual use of MOXIE_PC or the SPECIAL_REGS register class.  Could 
> they
> be deleted (with adjustments for decrementing MOXIE_CC)?

Yes, probably.

> 3. moxie_compute_frame () doesn't take !fixed_regs[regno] into account, which 
> the
> related loops in moxie_expand_prologue () and moxie_expand_epilogue ()
> do.  Bug?

Hmm.. looks like it.

> There are also some minor nits:
>
> 4. The comment above `size_for_adjusting_sp' states it's used in 
> expand_epilogue(),
> which it isn't.
>
> 5. The "Compute this since .." comment in moxie_initial_elimination_offset () 
> should
> probably refer to callee_saved_reg_size not local_vars_size, to match the 
> code.
>
> 6. There are two idential definitions of TRULY_NOOP_TRUNCATION(op,ip) in 
> moxie.h.
> The first one looks misplaced and should probably be deleted.

Thanks for all of this feedback.   I'm going to test some patches.

AG


On Sun, Jan 15, 2017 at 9:02 AM, Mikael Pettersson <mikpeli...@gmail.com> wrote:
> I have a toy backend based on the moxie backend as a template.  During its
> development I found some oddities in the moxie backend that may be bugs.
>
> 1. The REGNO_OK_FOR_INDEX_P(NUM) macro in moxie.h is:
>
> #define REGNO_OK_FOR_INDEX_P(NUM) MOXIE_FP
>
> Since MOXIE_FP is 0, this returns false for every register.  Should the body
> be a literal 0, or some comparison between NUM and MOXIE_FP?
>
> 2. I see no actual use of MOXIE_PC or the SPECIAL_REGS register class.  Could 
> they
> be deleted (with adjustments for decrementing MOXIE_CC)?
>
> 3. moxie_compute_frame () doesn't take !fixed_regs[regno] into account, which 
> the
> related loops in moxie_expand_prologue () and moxie_expand_epilogue () do.  
> Bug?
>
> There are also some minor nits:
>
> 4. The comment above `size_for_adjusting_sp' states it's used in 
> expand_epilogue(),
> which it isn't.
>
> 5. The "Compute this since .." comment in moxie_initial_elimination_offset () 
> should
> probably refer to callee_saved_reg_size not local_vars_size, to match the 
> code.
>
> 6. There are two idential definitions of TRULY_NOOP_TRUNCATION(op,ip) in 
> moxie.h.
> The first one looks misplaced and should probably be deleted.
>
>
> /Mikael

Reply via email to