This is the mail archive of the gcc-bugs@gcc.gnu.org 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]

[Bug target/70640] New: IEEE 128-bit floating point negative/abs has two thinkos


https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70640

            Bug ID: 70640
           Summary: IEEE 128-bit floating point negative/abs has two
                    thinkos
           Product: gcc
           Version: 6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: meissner at gcc dot gnu.org
  Target Milestone: ---

In applying the second fix for PR 70381, I noticed that the test float128-1.c
does not run on the pre-gcc7 branch, which is a branch I am collecting fixes to
be pushed upstream when GCC 7.0 opens up. One of the features of this branch is
that the default for register allocator was changed from the traditional reload
to LRA.

In looking at the test, it is generating incorrect code for IEEE 128-bit
floating point negate for power7/power8 systems (power9 is correct, since it
has hardware support).

If you compile the following file with -O2 -mvsx -mfloat128:

__float128 foo (__float128 a) { return -a; }

It will generate the following with reload:

        xxlorc 32,32,32
        xxlxor 33,33,33
        vslb 0,0,0
        vsldoi 0,0,1,15
        xxlxor 34,34,32
        blr

But with -mlra, it will generate:

        xxlxor 34,34,32
        blr

The reason is due to this insn:

(define_insn "*ieee_128bit_vsx_neg<mode>2_internal"
  [(set (match_operand:IEEE128 0 "register_operand" "=wa")
        (neg:IEEE128 (match_operand:IEEE128 1 "register_operand" "wa")))
   (use (match_operand:V16QI 2 "register_operand" "=v"))]
  "TARGET_FLOAT128 && !TARGET_FLOAT128_HW"
  "xxlxor %x0,%x1,%x2"
  [(set_attr "type" "vecsimple")])

In particular, the (use ...) has a constraint of "=v" when it should have "v".
LRA notices it is an output constraint, and discards the previous insns that
generate the -0.0q constant used by the negative operation.  In addition,
absolute value and negative absolute value have the same problem.

In addition, the expander for negative absolute value creates the wrong insn:

(define_insn_and_split "*ieee_128bit_vsx_nabs<mode>2"
  [(set (match_operand:IEEE128 0 "register_operand" "=wa")
        (neg:IEEE128
         (abs:IEEE128
          (match_operand:IEEE128 1 "register_operand" "wa"))))
   (clobber (match_scratch:V16QI 2 "=v"))]
  "TARGET_FLOAT128 && !TARGET_FLOAT128_HW && FLOAT128_IEEE_P (<MODE>mode)"
  "#"
  "&& 1"
  [(parallel [(set (match_dup 0)
                   (abs:IEEE128 (match_dup 1)))
              (use (match_dup 2))])]
{
  if (GET_CODE (operands[2]) == SCRATCH)
    operands[2] = gen_reg_rtx (V16QImode);

  operands[3] = gen_reg_rtx (V16QImode);
  emit_insn (gen_ieee_128bit_negative_zero (operands[2]));
}
  [(set_attr "length" "8")
   (set_attr "type" "vecsimple")])

I.e. it creates (abs ...) when it should create (neg (abs ...)).

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