[PATCH] Atom: Scheduler improvements for better imul placement

Igor Zamyatin izamyatin@gmail.com
Tue May 29 19:01:00 GMT 2012


Hi, Uros!

Sorry, I didn't realize that patch was missed. I attached new version.

Changelog:

2012-05-29  Yuri Rumyantsev  <yuri.s.rumyantsev@intel.com>

       * config/i386/i386.c (x86_sched_reorder): New function.
       Added new function x86_sched_reorder.

As for multiply modes, currently we handled most frequent case for
Atom and in the future this could be enhanced.

Thanks,
Igor

>
> Hello!
>
>> Ping?
>
> Please at least add and URL to the patch, it took me some time to found the latest version [1], I'm not even sure if it is the latest version...
>
> I assume that you cleared all issues with middle-end and scheduler maintainers, it is not clear from the message.
>
> +   (1) IMUL instrction is on the top of list;
>
> Typo above.
>
> +  static int issue_rate = -1;
> +  int n_ready = *pn_ready;
> +  rtx insn;
> +  rtx insn1;
> +  rtx insn2;
>
> Please put three definitions above on the same line.
>
> +  int i;
> +  sd_iterator_def sd_it;
> +  dep_t dep;
> +  int index = -1;
> +
> +  /* set up issue rate */
> +  if (issue_rate < 0)
> +    issue_rate = ix86_issue_rate();
>
> Please set issue_rate unconditionally here.  Also, please follow the GNU style of comments (Full sentence with two spaces after the dot) everywhere, e.g:
>
> /* Set up issue rate.  */
>
> +  if (!(GET_CODE (SET_SRC (insn)) == MULT
> +      && GET_MODE (SET_SRC (insn)) == SImode))
> +    return issue_rate;
>
> Is it correct that only SImode multiplies are checked against SImode multiplies? Can't we use DImode or HImode multiply (or other long-latency insns) to put them into the shadow of the first multiply insn?
>
> As proposed in [2], there are many other fine-tuning approaches proposed by the scheduler maintainer. OTOH, even the "big hammer"
> approach in the proposed patch makes things better, so it is the step in the right direction - and it is existing practice anyway.
>
> Under this rationale, I think that the patch should be committed to mainline. But please also consider proposed fine-tunings to refine the scheduling accuracy.
>
> So, OK for mainline, if there are no objections from other maintainers in next two days.
>
> [1] http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00964.html
> [2] http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00806.html
>
> Thanks,
> Uros.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: imul_reordering.patch
Type: application/octet-stream
Size: 3893 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20120529/55c90e1f/attachment.obj>


More information about the Gcc-patches mailing list