Re: i370 port

["Paul Edwards" <mutazilah@gmail.com>]

>> > 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.

This below was what I was hoping for ...

>> 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')"

My eyes lit up when I saw that!  However, it produced a compiler
error when I tried it.  But undeterred, I tried this:

(define_insn ""
  [(set (match_operand:SI 0 "register_operand" "=d")
^I(mult:SI (match_operand:SI 1 "register_operand" "0")
^I^I (match_operand:SI 2 "immediate_operand" "K")))]
  "(GET_CODE (operands[2]) == CONST_INT
   && REG_P (operands[0])
   && CONST_OK_FOR_LETTER_P (INTVAL (operands[2]), 'K'))"
  "*
{
  check_label_emit ();
  mvs_check_page (0, 4, 0);
  return \"MH^I%0,%H2\";
}"
   [(set_attr "length" "4")]
)

And it worked (verified by self-compile)!  And I relaxed the 
constraint on the "M" instruction as well.  Those old warnings 
are apparently irrelevant now.  Thank you sir.  :-)

> 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.)

Oh, I understand now.

> 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.

Right, with an "=".

> 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 ...

Yes indeed.

And we go to a lot of effort to maintain the length of that literal
pool, so we know when we need to break out into a new page.

That's what this does:

>>   mvs_check_page (0, 4, 0);

Although, as usual, it's broken.  Needs to be (0, 4, 2) for the
4-byte instruction followed by the 2 bytes it will use from the
literal pool.

> (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.)

I see.  That explains one of the difficulties of trying to get s390
instruction definitions and use them on i370.  People keep asking
why I don't "just" use the s390 ones.  If only life were that
simple.  :-)

> 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.)

That's putting it mildly.  :-)

Anyway, with that out of the way, I'll take a look at the next one.
There's only one bug remaining that I know of, and 3 workarounds
that I would like to reverse out and fix properly.

BFN.  Paul.