fnma patterns in ia64.md don't match with normal source code so generated code always uses separate multiply and subtract Release: 3.1.1 20020701 Environment: red hat 7.2 How-To-Repeat: gcc -O2 -S -funroll-loops -fforce-addr kcs.c shows several missed opportunities to employ fnma instructions
Fix: patch included for ia64.md which adds fnma patterns for SF, DF, and TF
From: Tim Prince <timothyprince@sbcglobal.net> To: Steven Bosscher <s.bosscher@student.tudelft.nl>, gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org, nobody@gcc.gnu.org, gcc-prs@gcc.gnu.org, tprince@computer.org Cc: Subject: Re: middle-end/7198: ia64.md missing usual fnma patterns Date: Sat, 1 Mar 2003 06:28:31 -0800 On Saturday 01 March 2003 04:01, Steven Bosscher wrote: > http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc& >pr=7198 > > Does this bug still exist in the 3.3 branch and mainline? It looks like > the patterns from your patch are already in ia64.md. > > Greetz > Steven That's interesting; I don't find them in the gcc-20030224 (3.3) snapshot. In fact, I just checked this week, and that snapshot was not employing fnma instructions in my test code, so I applied this patch again. The patterns which came in ia64.md require a sign change, to match the more common source code pattern, so it seems that a pattern with the sign change already applied is needed. I don't find my patterns in the gcc-3.2.2 either, so I just tried applying the patch (on my ia32 laptop here at home), and it appears to take successfully. My co-workers have not found a way to access CVS from our ia64 machines at the office, so I have always dealt with snapshots. Thanks for looking at this. Not exactly a bug, it's a missing optimization. -- Tim Prince
State-Changed-From-To: open->feedback State-Changed-Why: May have been fixed already for 3.3 and 3.4
From: Steven Bosscher <s.bosscher@student.tudelft.nl> To: gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org, nobody@gcc.gnu.org, gcc-prs@gcc.gnu.org, tprince@computer.org Cc: Subject: Re: middle-end/7198: ia64.md missing usual fnma patterns Date: Sat, 01 Mar 2003 13:01:06 +0100 http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=7198 Does this bug still exist in the 3.3 branch and mainline? It looks like the patterns from your patch are already in ia64.md. Greetz Steven
From: Steven Bosscher <s.bosscher@student.tudelft.nl> To: tprince@computer.org Cc: gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org, nobody@gcc.gnu.org Subject: Re: middle-end/7198: ia64.md missing usual fnma patterns Date: 01 Mar 2003 15:44:36 +0100 Op za 01-03-2003, om 15:28 schreef Tim Prince: > On Saturday 01 March 2003 04:01, Steven Bosscher wrote: > > http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc& > >pr=7198 > > > > Does this bug still exist in the 3.3 branch and mainline? It looks like > > the patterns from your patch are already in ia64.md. > That's interesting; I don't find them in the gcc-20030224 (3.3) snapshot. In > fact, I just checked this week, and that snapshot was not employing fnma > instructions in my test code, so I applied this patch again. The patterns > which came in ia64.md require a sign change, to match the more common source > code pattern, so it seems that a pattern with the sign change already applied > is needed. Ah, you're right, I overlooked the "neg:[SDT]F" part" in ia64.md.
From: Dara Hazeghi <dhazeghi@yahoo.com> To: gcc-gnats@gcc.gnu.org, tprince@computer.org Cc: Subject: Re: target/7198: [IA64] missing usual fnma patterns Date: Fri, 9 May 2003 11:34:36 -0700 http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit- trail&database=gcc&pr=7198 Hello, the log for this PR seems to indicate it was resolved. Is this true? Thanks, Dara
From: "Timothy C Prince" <tprince@myrealbox.com> To: dhazeghi@yahoo.com Cc: gcc-gnats@gcc.gnu.org, tprince@computer.org Subject: Re: Re: target/7198: [IA64] missing usual fnma patterns Date: Fri, 09 May 2003 22:18:02 +0000 -----Original Message----- From: Dara Hazeghi <dhazeghi@yahoo.com> To: gcc-gnats@gcc.gnu.org, tprince@computer.org Date: Fri, 9 May 2003 11:34:36 -0700 Subject: Re: target/7198: [IA64] missing usual fnma patterns http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=3Dview%20audit- trail&database=3Dgcc&pr=3D7198 Hello, the log for this PR seems to indicate it was resolved. Is this true? Thanks, Dara The last message from Steven in the log indicates that he agreed with me th= at nothing had been done with this. Tim Tim Prince
Should have been marked NEW per Tim's comments.
Still present. I'll ping the list...
The only copyright assignment I see for Tim Prince is for g77, and this is not a g77 patch. So technically I need an updated copyright assignment from him before I can accept his patch. The patch is pretty trivial though. I was able to write my own patch from the description of the problem before looking at Tim's patch. So another way to solve this problem is to write my own patch. This does need a bit of work to match current sources. XFmode is used instead of TFmode for the 80-bit FP format now. We should have a minus equivalent for every plus/neg pattern, of which there are 9. We already have 1 of them, so we need 8 new ones. It is probably better to replace the existing plus/neg pattern s rather than adding new ones, as I don't think the current ones can be generated except from the divide expanders. That requires verification. So that means deleting the SFmode plus/neg/mult pattern, and converting the other 8 patterns to use minus/mult. By the time I am done with that, I don't think I'll need a copyright assignment as I will have a completely new patch.
Subject: Re: [IA64] missing usual fnma patterns At 12:33 AM 12/16/2003, wilson at gcc dot gnu dot org wrote: >------- Additional Comments From wilson at gcc dot gnu dot org 2003-12-16 >08:32 ------- >The only copyright assignment I see for Tim Prince is for g77, and this is >not a >g77 patch. So technically I need an updated copyright assignment from him >before I can accept his patch. > >The patch is pretty trivial though. I was able to write my own patch from the >description of the problem before looking at Tim's patch. So another way to >solve this problem is to write my own patch. > >This does need a bit of work to match current sources. XFmode is used instead >of TFmode for the 80-bit FP format now. We should have a minus equivalent for >every plus/neg pattern, of which there are 9. We already have 1 of them, >so we >need 8 new ones. It is probably better to replace the existing plus/neg >pattern >s rather than adding new ones, as I don't think the current ones can be >generated except from the divide expanders. That requires verification. So >that means deleting the SFmode plus/neg/mult pattern, and converting the >other 8 >patterns to use minus/mult. By the time I am done with that, I don't >think I'll >need a copyright assignment as I will have a completely new patch. > >-- > > >http://gcc.gnu.org/bugzilla/show_bug.cgi?id=7198 > >------- You are receiving this mail because: ------- >You reported the bug, or are watching the reporter. Thanks. I noticed the change from TFmode to XFmode when I built gfortran-ssa recently, but I hadn't noticed the additional patterns. I didn't know about a distinction in the copyright assignment; I assumed that it was for all gcc events. Tim Prince
Subject: Candidate patch for PR 7198 Here's a candidate patch for PR 7198. I cannot bootstrap mainline due to what looks like an unrelated bug in cselib. However, the trivial test case double fnma(double a, double b, double c) { return a - b*c; } is compiled to fnma: .prologue .body .mfb nop 0 fnma.d f8 = f9, f10, f8 br.ret.sptk.many b0 exactly as desired. Also, I spot-checked that -minline-float-div-min-latency and -minline-float-div-max-throughput still work. zw * config/ia64/ia64.md (*nmaddsf4, *nmadddf4, *nmadddf4_alts) (*nmadddf4_trunc, *nmaddxf4, *nmaddxf4_truncsf, *nmaddxf4_truncdf) (*nmaddxf4_alts, *nmaddxf4_truncdf_alts): Rewrite pattern as (minus (op 3) (mult (op 1) (op 2))). Possibly rename pattern for consistency. Remove ??? comments suggesting that this be done. (*nmaddsf4_alts, *nmadddf4_truncsf_alts, *nmaddxf4_truncsf_alts): New patterns. (divsi3_internal, divdi3_internal_lat, divdi3_internal_thr) (divsf3_internal_lat, divsf3_internal_thr, sqrtsf2_internal_thr) (divdf3_internal_lat, divdf3_internal_thr, sqrtdf2_internal_thr) (divxf3_internal_lat, divxf3_internal_thr, sqrtxf2_internal_thr): Update to match.
Created attachment 5585 [details] pr7198.diff
Subject: Re: Candidate patch for PR 7198 On Mon, 2004-01-26 at 10:23, Zack Weinberg wrote: > Here's a candidate patch for PR 7198. I cannot bootstrap mainline due > to what looks like an unrelated bug in cselib. However, the trivial > test case There is a known problem with sched-ebb's use of cselib that results in accesses to memory that has already been freed. This causes bootstrap compare problems that go away if your environment size changes. However, I'd call this a sched-ebb bug rather than a cselib bug, because sched-ebb is using cselib in a way that it was never designed for. I've never seen this on my machine, and there have been so many other IA-64 problems to deal with that it has never made it to my high priority list of problems. This doesn't result in incorrect code, it only results in code that is scheduled differently. Thanks for fixing PR 7198. I don't really see a need for the nmaddsf4_alts pattern. This can only be created via expanders, and we currently don't have any expanders that need it. It doesn't hurt to have it though, and it does make the patterns more symmetrical, so I don't mind if you add it.
Subject: Re: Candidate patch for PR 7198 Jim Wilson <wilson@specifixinc.com> writes: > I don't really see a need for the nmaddsf4_alts pattern. This can only > be created via expanders, and we currently don't have any expanders that > need it. It doesn't hurt to have it though, and it does make the > patterns more symmetrical, so I don't mind if you add it. I may be adding a mechanism for specifying that all floating-point arithmetic operations are to use an alternative FPSR, in which case we need _alts patterns for all basic arithmetic patterns; that's why I added this. zw
Subject: Re: [IA64] missing usual fnma patterns zack at codesourcery dot com wrote: > I may be adding a mechanism for specifying that all floating-point > arithmetic operations are to use an alternative FPSR, in which case > we need _alts patterns for all basic arithmetic patterns; that's why I > added this. OK. If you do this, you might want to look at PR 7285 which is a vaguely related issue.
Fixed in 3.5.