Re: i370 port

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

Paul Edwards wrote:

> > 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.
> 
> Is there a way to give a predicate that just says "look at the
> constraint"?

Not that I'm aware of.

> It seems a bit overkill to add a new predicate
> for this one instruction.

As an alternative to the operand predicate, you might also add
an extra check to the insn condition.  For example, something
along the following lines should work:

 (define_insn ""
   [(set (match_operand:SI 0 "register_operand" "=d")
         (mult:SI (match_operand:SI 1 "register_operand" "0")
                  (match_operand:SI 2 "const_int_operand" "K")))]
   "CONST_OK_FOR_LETTER_P (INTVAL (operands[2]), 'K')"

> > As there is no other alternative,
> 
> No other alternative for this same pattern, right?  There was an
> alternative - the pattern that I explictly asked it to use, since
> I'd already done the K check in advance.

I was using "alternative" in the sense of multi-alternative
constraints within a single insn pattern, yes.  Reload will
never switch to use a different insn pattern, it will only
select one of the existing alternatives in the pattern.

> > and the insn supports neither memory nor register
> > operands, this is impossible for reload to fix up.
> 
> Hmmm.  I was wondering whether I could put a memory operand
> there, if that means it can fix it up regardless.  But that would
> give it the idea that it can put a fullword there, when a halfword
> operand is required, right?

Yes, the memory operand would have to be HImode in that case.

> > 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?
> 
> %H2 is ...
> 
> ;; Special formats used for outputting 370 instructions.
> ;;
> ;;   %H -- Print a signed 16-bit constant.

Yes, it prints an immediate *constant*.

> > If you indeed want to output immediate values here, you should
> 
> As opposed to wanting what?  All I want is the MH instruction
> to be available for use, so that when someone writes x = x * 5,
> it doesn't have to organize a register pair.

My point was that the MH instruction on an instruction set
architecture level *does not accept* an immediate operand,
but only a memory operand:

   MH     R1,D2(X2,B2)     [RX]

(There is a MULTIPLY HALFWORD IMMEDIATE (MHI) instruction as well, 
but I'm assuming you don't want to use it in the i370 port as that
instruction was added later on.)

So the usual way of using MH to multiply by an immediate value
is to place the constant into memory, typically some form of
literal pool.  But I see nothing in the i370 port that would
actually do that; instead, you seem to simply output the immediate
value itself into the assembler source.

If this works, it seems that the assembler will under the covers
manage a literal pool.  I was simply wondering if this is indeed
what you're relying on ...   (In the s390 port, the compiler will
always manage literal pools completely on its own and does never
rely on any assembler magic in that area.)

> e.g. I managed to make this:
> 
> (define_insn ""
>  [(set (match_operand:SI 0 "register_operand" "=d")
>        (mult:SI (sign_extend:SI (match_operand:HI 2 "memory_operand" "g"))
>                 (match_operand:SI 1 "register_operand" "0")))]
>  ""
>  "*
> {
>   check_label_emit ();
>   mvs_check_page (0, 4, 0);
>   return \"MH^I%0,%2\";
> }"
>    [(set_attr "length" "4")]
> )
> 
> produce:
> 
> C:\devel\gccnew\gcc>gccmvs -DUSE_MEMMGR -Os -S -ansi -pedantic-errors -DHAVE_CON
> FIG_H -DIN_GCC -DPUREISO -I ../../pdos/pdpclib -I . -I config/i370 -I 
> ../include
>          cfgloopanal.c
> cfgloopanal.c: In function `average_num_loop_insns':
> cfgloopanal.c:1379: error: unrecognizable insn:
> (insn 68 67 71 7 (set (reg:SI 45)
>         (mult:SI (reg:SI 44 [ <variable>.frequency ])
>             (const_int 10000 [0x2710]))) -1 (insn_list 67 (nil))
>     (expr_list:REG_DEAD (reg:SI 44 [ <variable>.frequency ])
>         (nil)))

Well, in this case someone has to push the constant into a literal pool.
You can either do this at expand time by calling force_const_mem, or else
you have to change the predicate to also accept immediates before reload
(then reload will do the force_const_mem for you).  (Note that if you in
fact do not manage a literal pool in the compiler today but rely on the
assembler, as discussed above, this whole approach may be difficult.)

Bye,
Ulrich

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