RFR(S): 8077843: adlc: allow nodes that use TEMP inputs in expand rules.

[Lindenmaier, Goetz]

HI Vladimir, 

the node generated in the Expand has a match rule itself.
I checked, all the TEMPs we use are in nodes with match rules.
Anyways, you get a syntax error if not:
"Syntax Error: <node>: TEMPs without match rule isn't supported"

We use nodes with match rules in expands a lot, as the match rule
is required for a lot of things. For example to generate the proper
ideal opcode, get it in the array must_clone etc.
If we don't want the nodes to be matched, we add predicate(false).
In our internal VM, we use nodes with TEMPs in expands on ia64 a lot.

For the code generated for the nodes I intend to contribute, see 
below.  

Best regards,
  Goetz.


// Card-mark for CMS garbage collection.
// This cardmark does an optimization so that it must not always
// do a releasing store. For this, it gets the address of
// CMSCollectorCardTableModRefBSExt::_requires_release as input.
// (Using releaseFieldAddr in the match rule is a hack.)
instruct storeCM_CMS(memory mem, iRegLdst releaseFieldAddr, flagsReg crx) %{
  match(Set mem (StoreCM mem releaseFieldAddr));
  effect(TEMP crx);
  predicate(false);
  ins_cost(MEMORY_REF_COST);

  // See loadConP.
  ins_cannot_rematerialize(true);

  format %{ "STB     #0, $mem \t// CMS card-mark byte (must be 0!), checking requires_release in [$releaseFieldAddr]" %}
  ins_encode( enc_cms_card_mark(mem, releaseFieldAddr, crx) );
  ins_pipe(pipe_class_memory);
%}

// Card-mark for CMS garbage collection.
// This cardmark does an optimization so that it must not always
// do a releasing store. For this, it needs the constant address of
// CMSCollectorCardTableModRefBSExt::_requires_release.
// This constant address is split off here by expand so we can use
// adlc / matcher functionality to load it from the constant section.
instruct storeCM_CMS_ExEx(memory mem, immI_0 zero) %{
  match(Set mem (StoreCM mem zero));
  predicate(UseConcMarkSweepGC);

  expand %{
    immL baseImm %{ 0 /* TODO: PPC port (jlong)CMSCollectorCardTableModRefBSExt::requires_release_address() */ %}
    iRegLdst releaseFieldAddress;
    flagsReg crx;
    loadConL_Ex(releaseFieldAddress, baseImm);
    storeCM_CMS(mem, releaseFieldAddress, crx);
  %}
%}


MachNode* storeCM_CMSNode::Expand(State* state, Node_List& proj_list, Node* mem) {
  Compile* C = Compile::current();
  // Add projection edges for additional defs or kills
  // TEMP crx
  MachTempNode *def;
  def = new MachTempNode(state->MachOperGenerator(FLAGSREG));
  add_req(def);

  return this;
}

MachNode* storeCM_CMS_ExExNode::Expand(State* state, Node_List& proj_list, Node* mem) {
  Compile* C = Compile::current();
  MachOper *op0 = new immLOper(
#line 6571 "/sapmnt/home1/d045726/oJ/8077838-ppc-hs-comp/src/cpu/ppc/vm/ppc.ad"
0 /* TODO: PPC port (jlong)CMSCollectorCardTableModRefBSExt::requires_release_address() */ 
#line 527 "../generated/adfiles/ad_ppc_64_expand.cpp"
);
  MachOper *op1 = new iRegLdstOper();
  MachOper *op2 = new flagsRegOper();
  MachNode *tmp0 = this;
  MachNode *tmp1 = this;
  MachNode *tmp2 = this;
  MachNode *tmp3 = NULL;
  MachNode *tmp4 = NULL;
  MachNode *tmp5 = NULL;
  unsigned num0 = 0;
  unsigned num1 = opnd_array(1)->num_edges();
  unsigned num2 = opnd_array(2)->num_edges();
  unsigned idx0 = oper_input_base();
  if (mem == (Node*)1) {
    idx0--; // Adjust base because memory edge hasn't been inserted yet
  }
  unsigned idx1 = idx0 + num0;
  unsigned idx2 = idx1 + num1;
  unsigned idx3 = idx2 + num2;
  MachNode *result = NULL;

  loadConL_ExNode *n0 = new loadConL_ExNode();
  n0->add_req(_in[0]);
  ((MachTypeNode*)n0)->_bottom_type = bottom_type();
  n0->set_opnd_array(0, state->MachOperGenerator(IREGLDST));
  tmp4 = n0;
  n0->set_opnd_array(1, op0->clone()); // baseImm
  if(tmp3 != NULL)
    n0->add_req(tmp3);
  result = n0->Expand( state, proj_list, mem );

  storeCM_CMSNode *n1 = new storeCM_CMSNode();
  n1->add_req(_in[0]);
  ((MachTypeNode*)n1)->_bottom_type = bottom_type();
  n1->set_opnd_array(0, state->MachOperGenerator(UNIVERSE));
  if (mem != (Node*)1) {
    n1->add_req(_in[1]);	// Add memory edge
  }
  n1->set_opnd_array(1, opnd_array(1)->clone()); // mem
  if(tmp1 == this) {
    for(unsigned i = 0; i < num1; i++) {
      n1->add_req(_in[i + idx1]);
    }
  }
  else n1->add_req(tmp1);
  tmp1 = n1;
  n1->set_opnd_array(2, op1->clone()); // releaseFieldAddress
  if(tmp4 != NULL)
    n1->add_req(tmp4);
  n1->set_opnd_array(3, op2->clone()); // crx
  if(tmp5 != NULL)
    n1->add_req(tmp5);
  result = n1->Expand( state, proj_list, mem );


  return result;
}



-----Original Message-----
From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On Behalf Of Vladimir Kozlov
Sent: Freitag, 17. April 2015 02:28
To: hotspot-dev at openjdk.java.net
Subject: Re: RFR(S): 8077843: adlc: allow nodes that use TEMP inputs in expand rules.

Here is what Tom R said during review of TEMP implementation:

 >> 3. In formssel.cpp you restricted TEMP usage only to instruction
 >> with match rule, why? What about effect()?:
 >> + bool InstructForm::has_temps() {
 >> +   if (_matrule) {
 >
 >TEMP isn't allowed for instructions which don't have match rules.  The 
 >reason is that if an instruct doesn't have a match rule the only way 
 >it can be constructed is by an expand rule.  expand rules look like 
 >explicit invocations of encodings but TEMPs are synthetic so you can't 
 >properly write an expand rule to work with TEMP.  I mean you could 
 >make it work but I decided not to support it.  I'll make this more 
 >explicit.

So I am not sure that just removing the assert will be enough. Please, 
verify.

Regards,
Vladimir

On 4/15/15 6:08 AM, Lindenmaier, Goetz wrote:
> Hi,
>
> I have a tiny fix that allows nodes that use TEMP inputs / TEMP effect in
> expand rules.
>
> Currently an assertion fires if you do so.
> This is harmless, though, as the TEMP node is added in the Expand() of the used node,
> which is called by the Expand() of the node being expanded.
> Probably the assertion was meant for the node being expanded.
>
> The change simply removed the assertion.
>
> Please review this change.  I please need a sponsor.
> http://cr.openjdk.java.net/~goetz/webrevs/8077843-adlcTEMP/webrev.01/
>
> I need this feature for a change I intend to do in the ppc.ad file.
>
> Best regards,
>    Goetz.
>