Bug 58838 - mullw sets condition code incorrectly.
mullw sets condition code incorrectly.
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: target
4.9.0
: P3 normal
: 4.9.0
Assigned To: Not yet assigned to anyone
: wrong-code
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-22 07:45 UTC by Doug Kwan
Modified: 2013-10-24 13:26 UTC (History)
2 users (show)

See Also:
Host: x86_64-linux-gnu
Target: powerpc64-linux-gnu
Build: x86_64-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2013-10-22 00:00:00


Attachments
Disable mullw. dot form in 64 bit mode (1.09 KB, patch)
2013-10-22 18:19 UTC, David Edelsohn
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Doug Kwan 2013-10-22 07:45:42 UTC
It was observed in 4.7 and trunk that the Power backend generated bad code involving condition setting mullw.  Below is a test case for the problem:

-----
#include <cassert>
#include <cstdlib>
#include <vector>

struct b88 {
 char data[88];
};

namespace {

inline int int_size(const std::vector<b88>& v) {
 return v.size();
}

}

int __attribute__ ((noinline))
foo(const std::vector<b88>& v) {
 if (int_size(v) > 0) {
   return atoi("1");
 }
 return 0;
}

int main() {
 std::vector<b88> v;
 v.push_back(b88());
 assert(foo(v) != 0);
 return 0;
}
----

The above test failed with gcc 4.7 and trunk for target powerpc64-linux-gnu

We seem to be fine getting out of the middle-end of gcc-4.7:

;; Function int foo(const std::vector<b88>&) (_Z3fooRKSt6vectorI3b88SaIS0_EE, funcdef_no=473, decl_uid=9136, cgraph_uid=102)

int foo(const std::vector<b88>&) (const struct vector & v)
{
 int _1;
 struct b88 * _4;
 struct b88 * _5;
 long int _6;
 long int _7;
 long int _8;
 long int _9;
 int _10;
 long int _11;
 int _12;

 <bb 2>:
 _4 = MEM[(const struct vector *)v_3(D)];
 _5 = MEM[(const struct vector *)v_3(D) + 8B];
 _6 = (long int) _5;
 _7 = (long int) _4;
 _8 = _6 - _7;
 _9 = _8 /[ex] 88;
 _10 = (int) _9;
 if (_10 > 0)
   goto <bb 3>;
 else
   goto <bb 4>;

 <bb 3>:
 _11 = strtol ("1", 0B, 10);
 _12 = (int) _11;

 <bb 4>:
 # _1 = PHI <_12(3), 0(2)>
 return _1;

}

RTL expansion looks correct, gcc expands "_8 /[ex] 88;" into shift and multiplication.

(_8 >> 3) * 0x2e8ba2e8ba2e8ba3.

Since we know that the _8 is a multiple of 88, we can do this. 0x2e8ba2e8ba2e8ba3 is the multiplicative-inverse of 11 in Z{2^64}.


But the selected PPC instruction is bad:

  0x10000b80 <._Z3fooRKSt6vectorI3b88SaIS0_EE>:        lis     r9,11915
  0x10000b84 <._Z3fooRKSt6vectorI3b88SaIS0_EE+4>:      ld      r8,8(r3)
  0x10000b88 <._Z3fooRKSt6vectorI3b88SaIS0_EE+8>:      ld      r10,0(r3)
  0x10000b8c <._Z3fooRKSt6vectorI3b88SaIS0_EE+12>:     ori     r9,r9,41704
  0x10000b90 <._Z3fooRKSt6vectorI3b88SaIS0_EE+16>:     rldicr  r9,r9,32,31
  0x10000b94 <._Z3fooRKSt6vectorI3b88SaIS0_EE+20>:     subf    r10,r10,r8
  0x10000b98 <._Z3fooRKSt6vectorI3b88SaIS0_EE+24>:     oris    r9,r9,47662
  0x10000b9c <._Z3fooRKSt6vectorI3b88SaIS0_EE+28>:     sradi   r10,r10,3
  0x10000ba0 <._Z3fooRKSt6vectorI3b88SaIS0_EE+32>:     ori     r9,r9,35747
  0x10000ba4 <._Z3fooRKSt6vectorI3b88SaIS0_EE+36>:     mullw.  r8,r10,r9  <================ note the "dot"
  0x10000ba8 <._Z3fooRKSt6vectorI3b88SaIS0_EE+40>:     ble     0x10000bf0 <._Z3fooRKSt6vectorI3b88SaIS0_EE+112>

mullw correctly computes the lower 32-bits of  "_9 = _8 /[ex] 88;" but it sets the condition code incorrectly, since signed-comparison is done in 64 bits.  Instead there should be sign extension, ie.

mullw  r8,r10,r9
extsw. r8,r8
Comment 1 David Edelsohn 2013-10-22 18:11:03 UTC
Confirmed. The backend should not generate mullw. in 64 bit mode.
Comment 2 David Edelsohn 2013-10-22 18:13:14 UTC
Index: rs6000.md
===================================================================
--- rs6000.md   (revision 203930)
+++ rs6000.md   (working copy)
@@ -2699,7 +2699,7 @@
                             (match_operand:SI 2 "gpc_reg_operand" "r,r"))
                    (const_int 0)))
    (clobber (match_scratch:SI 3 "=r,r"))]
-  ""
+  "TARGET_32BIT"
   "@
    mullw. %3,%1,%2
    #"
@@ -2727,7 +2727,7 @@
                    (const_int 0)))
    (set (match_operand:SI 0 "gpc_reg_operand" "=r,r")
        (mult:SI (match_dup 1) (match_dup 2)))]
-  ""
+  "TARGET_32BIT"
   "@
    mullw. %0,%1,%2
    #"
Comment 3 David Edelsohn 2013-10-22 18:19:35 UTC
Created attachment 31071 [details]
Disable mullw. dot form in 64 bit mode

Revised patch that also disables splitters.
Comment 4 David Edelsohn 2013-10-22 18:20:15 UTC
Add keyword and target.
Comment 5 David Edelsohn 2013-10-23 14:32:35 UTC
Author: dje
Date: Wed Oct 23 14:32:32 2013
New Revision: 203977

URL: http://gcc.gnu.org/viewcvs?rev=203977&root=gcc&view=rev
Log:
        PR target/58838
        * config/rs6000/rs6000.md (mulsi3_internal1 and splitter): Add
        TARGET_32BIT final condition.
        (mulsi3_internal2 and splitter): Same.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.md
Comment 6 David Edelsohn 2013-10-24 12:53:54 UTC
Author: dje
Date: Thu Oct 24 12:53:51 2013
New Revision: 204009

URL: http://gcc.gnu.org/viewcvs?rev=204009&root=gcc&view=rev
Log:
        Backport from mainline
        2013-10-23  David Edelsohn  <dje.gcc@gmail.com>

        PR target/58838
        * config/rs6000/rs6000.md (mulsi3_internal1 and splitter): Add
        TARGET_32BIT final condition.
        (mulsi3_internal2 and splitter): Same.

Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/rs6000/rs6000.md
Comment 7 David Edelsohn 2013-10-24 13:26:57 UTC
Patches applied to trunk and 4.8 branches.