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: RFA: MN10300: Replace cc0 with CC_REG


Hi Richard,

  Thanks for the review.  Here are some answers to specific points:

> There are unrelated changes in here.  It would have been easier if
> you'd have done some of the other cleanup in separate patches.

Sorry about that.  I am working from a code base which has lots and lots
of local changes...  I have removed the worst offenders from the revised
patch however.

> >   { 0x3fdff, 0 }, 	/* GENERAL_REGS */	\
> Not a bug, but it would be helpful if these lined up ala %#08x.

Done.

> > -		 (match_test "XEXP (op, 0) != stack_pointer_rtx"))
> > +		 (match_test "REGNO (XEXP (op, 0)) != STACK_POINTER_REGNUM"))
> Why?  The correlation is guaranteed.

Old fix from when the correlation could not be guaranteed.  Removed from
the revised patch.

> > +(define_expand "mulsidi3"
> < [snip]
> > +(define_insn "*mulsidi3_insn"
> Is there a reason to have the separate expander?  The same comment
> appears to apply to several other of the basic named patterns.  As
> distinguished from those that have both AM33 and non-am33
> implementations.

You are right, they are not needed ... yet.  They are needed in a patch
to come, but I have removed them for now.


> > +(define_insn_and_split "*cbranchsi4_<code>"
> > +  [(set (pc)
> > +	(if_then_else (most_cond (match_operand:SI 
> Any reason to use macros here instead of match_operator?  It seems a 
> bit wasteful to generate 8 copies of these patterns...

Indeed.  Fixed.


> > +;; We expand the comparison into a single insn so that it will not be split
> > +;; up by reload.
> >  (define_expand "cbranchsi4"
> Do you not have an add that doesn't clobber flags?

No. :-(

> Are you planning to eliminate redundant compares via peepholes?

Eventually yes, but not until a few more patches have gone in.


> >  (define_insn_and_split "mn10300_loadPC"
> >    [(parallel
> >      [(set (reg:SI PIC_REG) (pc))
> >       (use (match_operand 0 "" ""))])]
> > -  ""
> > +  "! TARGET_AM33"
> >    "#"
> >    "reload_completed"
> >    [(match_operand 0 "" "")]
> > -  "
> >  {
> > +    if (TARGET_AM33)
> > +      emit_insn (gen_am33_loadPC (operands[0]));
> > +    else
> > +      {
> Err, I think this should be "&& reload_completed".
> That should cleanup the confusing TARGET_AM33 test
> inside a pattern that's !TARGET_AM33.

Thanks - fixed.


So, I have attached a revised patch.  There are still no regressions
against an mn10300-elf toolchain.  OK to apply ?

Cheers
  Nick

gcc/ChangeLog
2010-10-11  Nick Clifton  <nickc@redhat.com>

	* config/mn10300/mn10300.h (CONSTANT_ALIGNMENT): Define.
	(DATA_ALIGNMENT, LOCAL_ALIGNMENT): Define.
	(FIRST_PSEUDO_REGISTER): Increase by one.
	(FIXED_REGISTERS, CALL_USED_REGISTERS): Update with CC_REG.
	(HARD_REGNO_MODE_OK): Call mn10300_hard_regno_mode_ok.
	(MODES_TIEABLE): Call mn10300_modes_tieable.
	(REG_CLASS_NAMES, REG_CLASS_CONTENTS, REGNO_REG_CLASS): Add
	CC_REGS.
	(LEGITIMATE_CONSTANT_P): Call mn10300_legitimate_constant_p.
	(CC_OVERFLOW_UNUSABLE, CC_NO_CARRY, NOTICE_UPDATE_CC)
	(SELECT_CC_MODE, REVERSIBLE_CC_MODE): Delete.
	(REGISTER_NAMES, ADDITIONAL_REGISTER_NAMES): Add CC register.
	(ASM_OUTPUT_REG_PUSH, ASM_OUTPUT_REG_POP): Delete.
	(mn10300_cc_status_mdep): Delete.
	(CC_STATUS_MDEP, CC_STATUS_MDEP_INIT): Delete.
	* config/mn10300/mn10300 (mn10300_option_override): Stop disabling
	the combine-stack-adjust pass.
	(print_operand): Use the mode of the comparison operation to
	select the comparison suffix.
	(notice_update_cc): Delete.
	(mn10300_secondary_reload_class): Remove test for stack pointer
	based arithmetic.
	(output_tst): Rename to mn10300_output_cmp.
	(impossible_plus_operand): Move into predicates.md.
	(mn10300_legitimize_address): Make static.
	(mn10300_legitimate_address_p): Make static.  Only allow SI sized
	constant pic operands.
	(mn10300_legitimate_constant_p): New function.
	(mn10300_case_values_threshold): Make static.
	(mn10300_hard_regno_mode_ok): New function.
	(mn10300_modes_tieable): New function.
	(mn10300_select_cc_mode): New function.
	* config/mn10300/predicates.md (impossible_plus_operand): Define.
	* config/mn10300/mn10300-protos.h: Tidy.
	(mn10300_legitimate_constant_p, mn10300_modes_tieable)
	(mn10300_hard_regno_mode_ok, mn10300_select_cc_mode): Prototype.
	* config/mn10300/mn10300.md (cc attribute): Delete.  Replace
	with clobbers or sets of CC_REG.
	(CC_REG): Define.
	(mov*): Remove use of CLR instruction.
	(cbranch_si4_<code>): New pattern/split.
	(integer_conditional_branch): New pattern.
	(cbranch_sf4_<code>): New pattern/split.
	(float_conditional_branch): New	pattern.
	(casesi): Use addsi3 pattern instead of movsi pattern to add and
	move a value at the same time.
	(cc0 peepholes): Remove.

Attachment: mn10300.cc0.patch.2.bz2
Description: BZip2 compressed data


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