[PATCH] Add new target h8300-*-linux

Yoshinori Sato ysato@users.sourceforge.jp
Tue Apr 21 06:04:00 GMT 2015


At Mon, 20 Apr 2015 09:26:21 -0600,
Jeff Law wrote:
> 
> On 04/19/2015 10:51 PM, Yoshinori Sato wrote:
> >>> +  if (TARGET_H8300H && (TARGET_H8300S || TARGET_H8300SX))
> >>> +    {
> >>> +      target_flags ^= MASK_H8300H;
> >>> +    }
> >> I'm a bit concerned by this.  Why did you need to make this change?
> >> 
> > 
> > The flag is exclusion, but it's set both.
> Hmmm, IIRC the port has many places where it may assume that H8300H is
> set for H8300S/H8300SX.  I did a very quick audit and saw:

I think it's being broken by my changes, so it's checked.

> I would recommend reviewing the extzv_16_8 pattern which has the
> condition "TARGET_H8300H" and changing the condition to
> "TARGET_H8300H || TARGET_H8300S"  since AFAICT that pattern should
> work on both processor variants.
> 
> Similarly there's two peephole patterns have have conditions that
> looks like this:
> 
>  "(TARGET_H8300H || TARGET_H8300S)
>     && peep2_reg_dead_p (1, operands[0])
>     && ((TARGET_H8300H && INTVAL (operands[1]) == 3)
>          || INTVAL (operands[1]) == 7
>          || INTVAL (operands[1]) == 15
>          || INTVAL (operands[1]) == 31
>          || INTVAL (operands[1]) == 63
>          || INTVAL (operands[1]) == 127
>          || INTVAL (operands[1]) == 255)"
> 
> 
> I'm pretty sure the second TARGET_H8300H should be (TARGET_H8300H ||
> TARGET_H8300S).

OK.

> 
> In h8300.c::get_shift_alg, case HIshift, count 14, does this need to change?
>

It loock unnecessary.
Remove it.

>             else if (TARGET_H8300H)
>                 {
>                   info->special =
> "shll.b\t%t0\n\tsubx.b\t%s0,%s0\n\tshll.b\t%t0\n\trotxl.b\t%s0\n\texts.w\t%T0";
>                   info->cc_special = CC_SET_ZNV;
>                 }
>               else /* TARGET_H8300S */
>                 gcc_unreachable ();
> 
> Similarly SImode shifts by 28-30 bits should be reviewed in a similar
> manner.  As should the implementation of h8300_shift_needs_scratch_p.
> 
> output_a_rotate also needs to be reviewed if you want to make the
> change to turn off H8300H when H8/S is true.  Similarly for
> compute_a_rotate_length.
>

The one put in by an experiment was left.
Removed.

> 
> There may be others, these are what I found with a very quick
> search. If there's not a compelling reason to make the change, I'd
> recommend against it.
>

OK.
I will fix and resent.

> 
> 
> Jeff

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>



More information about the Gcc-patches mailing list