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 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).
In h8300.c::get_shift_alg, case HIshift, count 14, does this need to change?
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.
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.
Jeff
