Re: i370 port

["Ulrich Weigand" <uweigand@de.ibm.com>]

Paul Edwards wrote:

> Hi Ulrich.  Thanks for the reply.  I didn't use gcc_assert because I
> didn't see it defined anywhere, but the rest of the fix worked fine.

Ah, I see this macro was only added in 4.x.  In 3.x you should just
use abort () directly, like so:

  if (GET_CODE (dest) != LABEL_REF)
    abort ();

> (*) I had to disable the MH (multiply halfword) instruction in order
> to get it to go through unfortunately.  See below (+).
 
> (+) Can you spot anything wrong with this?

> ;(define_insn ""
> ;  [(set (match_operand:SI 0 "register_operand" "=d")
> ;       (mult:SI (match_operand:SI 1 "register_operand" "0")
> ;                (match_operand:SI 2 "immediate_operand" "K")))]
> ;  ""
> ;  "*
> ;{
> ;  check_label_emit ();
> ;  mvs_check_page (0, 4, 0);
> ;  return \"MH  %0,%H2\";
> ;}"
> ;   [(set_attr "length" "4")]
> ;)

The combination of predicates and constraints on this insn is broken.

Before reload, the predicate "immediate_operand" explicitly allows
*any* SImode immediate value.  However, during reload, the "K"
constraint accepts only a subset of values.  As there is no other
alternative, and the insn supports neither memory nor register
operands, this is impossible for reload to fix up.

In addition, I don't quite understand how this pattern works in
the first place; MH requires a memory operand, but this pattern
seems to output an immediate value as operand.  Is there some
magic going on in your assembler?

If you indeed want to output immediate values here, you should
probably define a new *predicate* that constrains the set of
allowed values even before reload.

In the s390 port, we're instead modelling the MH instruction
with a memory operand (this still allows the generic parts of
GCC to push immediate operands into memory, if they are in
range for an HImode operand):

(define_insn "*mulsi3_sign"
  [(set (match_operand:SI 0 "register_operand" "=d")
        (mult:SI (sign_extend:SI (match_operand:HI 2 "memory_operand" "R"))
                 (match_operand:SI 1 "register_operand" "0")))]
  ""
  "mh\t%0,%2"
  [(set_attr "op_type"  "RX")
   (set_attr "type"     "imul")])

> ; See mulsi3 comment above as to why this is constrained to
> ; "di" rather than "g"
> (define_insn ""
>   [(set (match_operand:DI 0 "register_operand" "=d")
>         (mult:DI (match_operand:DI 1 "general_operand" "0")
>                  (match_operand:SI 2 "general_operand" "di")))]
>   ""
>   "*
> {
>   check_label_emit ();
>   if (REG_P (operands[2]))
>     {
>       mvs_check_page (0, 2, 0);
>       return \"MR       %0,%2\";
>     }
>   mvs_check_page (0, 4, 0);
>   return \"M    %0,%2\";
> }"
>    [(set_attr "length" "4")]
> )

This also seems broken.  A MULT:DI must have two DImode operands,
it cannot have one DImode and one SImode operand.  Also, it is in
fact incorrect that it takes the full DImode first operand; rather,
it only uses the low 32-bit of its first operand as input.

In the s390 port we're modelling the real behavior of the instruction
using two explicit SIGN_EXTEND codes:

(define_insn "mulsidi3"
  [(set (match_operand:DI 0 "register_operand" "=d,d")
        (mult:DI (sign_extend:DI
                   (match_operand:SI 1 "register_operand" "%0,0"))
                 (sign_extend:DI
                   (match_operand:SI 2 "nonimmediate_operand" "d,R"))))]
  "!TARGET_64BIT"
  "@
   mr\t%0,%2
   m\t%0,%2"
  [(set_attr "op_type"  "RR,RX")
   (set_attr "type"     "imul")])

> Regardless, when that code is NOT commented out, such that I get that
> error, it is surprising that it is passing through this code:
> 
>       emit_insn (gen_rtx_SET (VOIDmode, r,
>                           gen_rtx_MULT (DImode, r, operands[2])));
> 
> where it is clearly attempting to do a DImode multiply, and thus
> shouldn't be matching the MH, that I am getting the problem.

Well, the point of optimization is that the RTXes do not stay the
way they were originally expanded ...   The optimizers will attempt
to perform various generic optimization on the code, and if the
back-end claims to support a pattern that implements any of those
optimized forms, it will get used.  In this case, even though you
expanded a DImode multiply, common code may notice that it can
be optimized to a SImode multiply instead.

Generally speaking, your RTX patterns *must* be fully correct and
represent the actual behavior of the machine in all cases.  If there
are corner cases formally allowed by the RTX pattern, but the
behavior of the machine differs, this may cause breakage.  Even if
your expanders avoid those corner cases when using your patterns,
this will not be true for the optimizers.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com