Bug 48673 - [4.7 Regression] GCC generates WAW and RAW conflicts on IA64.
Summary: [4.7 Regression] GCC generates WAW and RAW conflicts on IA64.
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.7.0
: P3 normal
Target Milestone: 4.7.0
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2011-04-18 22:28 UTC by Steve Ellcey
Modified: 2011-06-09 13:56 UTC (History)
5 users (show)

See Also:
Host:
Target: ia64-*-hpux
Build:
Known to work:
Known to fail:
Last reconfirmed: 2011-04-28 11:48:26


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Ellcey 2011-04-18 22:28:14 UTC
Starting with version r171842, the tests gfortran.dg/sms-1.f90 and gfortran.dg/sms-2.f90 are failing on IA64 HP-UX and Linux.  The failures
are excess messages during compilation like this:

$ 1.23-trunk/bin/gfortran -c sms-2.s                                 <
sms-2.s: Assembler messages:
sms-2.s:413: Warning: Use of 'ldfs' violates RAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 15
sms-2.s:413: Warning: Only the first path encountering the conflict is reported
sms-2.s:411: Warning: This is the location of the conflicting usage

The assembly code generated by GCC for sms-2.f90 has:

.L9:
        ;;
        addp4 r15 = 0,r14
        fadd.s f6 = f6, f7
        ldfs f7 = [r15]
        adds r14 = 4, r14
        br.cloop.sptk.few .L9
        ;;

So we are changing r15 and using it in the same bundle and we should not be
doing that.   This could cause us to generate incorrect results.

To reproduce this you can compile sms-1.f90 with '-O2 -funroll-loops -fmodulo-sched' or sms-2.f90 with '-O2 -fmodulo-sched'.

I am not sure if the problem is in the general scheduling code or in the IA64 specific target code.
Comment 1 Andrey Belevantsev 2011-04-27 08:01:19 UTC
Steve, can you reproduce this on the latest trunk?  I have tried to look at this but I cannot reproduce the problem at 173004.
Comment 2 Steve Ellcey 2011-04-27 21:58:53 UTC
It looks like I was wrong about this showing up on IA64 Linux.  I am only seeing it on IA64 HP-UX and only in 32 bit mode.  That probably means it is related to the HP-UX pointer extension (addp4).  Do you have access to an HP-UX system?
Comment 3 Alexander Monakov 2011-04-28 11:48:26 UTC
The problem can be seen by visual inspection of generated assembly with a cross-compiler configured with --target=ia64-hpux --enable-languages=c,fortran.

The issue must have appeared with this patch:

Author: bernds <bernds@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Fri Apr 1 17:42:35 2011 +0000
        * sched-ebb.c (schedule_ebbs): Honor the BB_DISABLE_SCHEDULE flag.

After that patch Haifa scheduler would not place TImodes on cycle boundaries, as such basic blocks are not touched by the scheduler at all, but the back-end relies on TImodes for placement of stop bits.  Selective scheduler handles BB_DISABLE_SCHEDULE flag by skipping scheduling but still computing cycle boundaries and calling target hooks in sel_region_target_finish.
Comment 4 Bernd Schmidt 2011-05-05 18:45:59 UTC
Sounds like either ia64 must clear the disable-schedule flag on all basic blocks, or sms must be fixed to set TImode on the insns as needed.
Comment 5 Andrey Belevantsev 2011-05-06 06:24:31 UTC
(In reply to comment #4)
> Sounds like either ia64 must clear the disable-schedule flag on all basic
> blocks, or sms must be fixed to set TImode on the insns as needed.

Or schedule_block could have a mode of preserving the existing insn order, which can also be useful for testing purposes.  If this sounds like an overkill, something like sel-sched.c:reset_sched_cycles_in_current_ebb could be adapted to work.  This function feeds the bb insns to DFA but also follows the old schedule w.r.t. to dependency latencies.  Not having the old schedule would mean that we would honor only DFA latencies reflected in insn cycles and TImodes, but this may be enough.
Comment 6 Bernd Schmidt 2011-05-06 12:09:32 UTC
(In reply to comment #5)
> Or schedule_block could have a mode of preserving the existing insn order,
> which can also be useful for testing purposes.  If this sounds like an
> overkill, something like sel-sched.c:reset_sched_cycles_in_current_ebb could be
> adapted to work.  This function feeds the bb insns to DFA but also follows the
> old schedule w.r.t. to dependency latencies.  Not having the old schedule would
> mean that we would honor only DFA latencies reflected in insn cycles and
> TImodes, but this may be enough.

That sounds fragile. The DFA is not sufficient to produce an accurate schedule on all targets. If SMS produces a schedule it might as well mark it with accurate uses of TImode.
Comment 7 Andrey Belevantsev 2011-05-06 12:33:39 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > Or schedule_block could have a mode of preserving the existing insn order,
> > which can also be useful for testing purposes.  If this sounds like an
> > overkill, something like sel-sched.c:reset_sched_cycles_in_current_ebb could be
> > adapted to work.  This function feeds the bb insns to DFA but also follows the
> > old schedule w.r.t. to dependency latencies.  Not having the old schedule would
> > mean that we would honor only DFA latencies reflected in insn cycles and
> > TImodes, but this may be enough.
> 
> That sounds fragile. The DFA is not sufficient to produce an accurate schedule
> on all targets. If SMS produces a schedule it might as well mark it with
> accurate uses of TImode.

But sms works too early.  There is no guarantee that TImode markers will live until machreorg, where we need them.  Also, for the before reload scheduler there is no guarantee the schedule would be fully accurate.

If you are saying that just DFA is not enough for making proper TImodes, then IMHO the only choice is to have the full run of schedule_block preserving insn order and taking into account all of DFA/dependency/target hook stuff just before the machreorg pass.
Comment 8 Bernd Schmidt 2011-06-09 13:55:44 UTC
Author: bernds
Date: Thu Jun  9 13:55:41 2011
New Revision: 174844

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174844
Log:
	PR target/48673
	* config/ia64/ia64.c (ia64_reorg): Clear BB_DISABLE_SCHEDULE flag
	in all basic blocks.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/ia64/ia64.c
Comment 9 Bernd Schmidt 2011-06-09 13:56:05 UTC
Fixed.