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
<ys...@users.sourceforge.jp>
