This is the mail archive of the 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: [PATCH] Add new target h8300-*-linux

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.


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