diff mbox

Add post_expand_call_insn hook

Message ID 535F6A00.9050906@mentor.com
State New
Headers show

Commit Message

Tom de Vries April 29, 2014, 8:59 a.m. UTC 
On 24-04-14 17:13, Eric Botcazou wrote:
>> The hook is called right after expansion of calls, and allows a target to do
>> additional processing, such as f.i. adding clobbers to
>> CALL_INSN_FUNCTION_USAGE.
>>
>> Instead of using the hook, we could add code to the preparation statements
>> operand of the different call expands, but that requires those expands not
>> to use the rtl template, and generate all the rtl through c code. Which
>> requires a rewrite of the call expands in case of Aarch64.
>
> If Aarch64 is the only problematic back-end, then it should be changed to do
> what the other back-ends already do to use use_reg.  IMO adding such a bogus
> hook should be the very last resort solution.
>

Eric,

I've written this concept patch, which tries to address the same problem, but in 
a different (and I hope more generic) way.

It adds a post-emission C-code operand to define_expand.

As an example of how this could be useful, for the define_expand of call and 
call_value in the arm target, I'm using the new operand to do the post-emit call 
processing done currently in arm_emit_call_insn. This allows us to eliminate the 
call_internal and call_value_internal define_expands, and simplifies the call 
and call_value define_expands.

Any comments?

Thanks,
- Tom

Comments

Richard Henderson April 29, 2014, 6:56 p.m. UTC | #1 
On 04/29/2014 01:59 AM, Tom de Vries wrote:
> On 24-04-14 17:13, Eric Botcazou wrote:
>>> The hook is called right after expansion of calls, and allows a target to do
>>> additional processing, such as f.i. adding clobbers to
>>> CALL_INSN_FUNCTION_USAGE.
>>>
>>> Instead of using the hook, we could add code to the preparation statements
>>> operand of the different call expands, but that requires those expands not
>>> to use the rtl template, and generate all the rtl through c code. Which
>>> requires a rewrite of the call expands in case of Aarch64.
>>
>> If Aarch64 is the only problematic back-end, then it should be changed to do
>> what the other back-ends already do to use use_reg.  IMO adding such a bogus
>> hook should be the very last resort solution.
>>
> 
> Eric,
> 
> I've written this concept patch, which tries to address the same problem, but
> in a different (and I hope more generic) way.
> 
> It adds a post-emission C-code operand to define_expand.
> 
> As an example of how this could be useful, for the define_expand of call and
> call_value in the arm target, I'm using the new operand to do the post-emit
> call processing done currently in arm_emit_call_insn. This allows us to
> eliminate the call_internal and call_value_internal define_expands, and
> simplifies the call and call_value define_expands.


Is this patch really any better?  I can't see that it is myself.  It seems to
me that the existing mechanism to emit the call, then append to FUNCTION_USAGE
is perfectly clear.  This new argument to define_expand seems less clear.

What are you trying to fix, anyway?


r~
Tom de Vries April 29, 2014, 11 p.m. UTC | #2 
On 29-04-14 20:56, Richard Henderson wrote:
>> I've written this concept patch, which tries to address the same problem, but
>> >in a different (and I hope more generic) way.
>> >
>> >It adds a post-emission C-code operand to define_expand.
>> >
>> >As an example of how this could be useful, for the define_expand of call and
>> >call_value in the arm target, I'm using the new operand to do the post-emit
>> >call processing done currently in arm_emit_call_insn. This allows us to
>> >eliminate the call_internal and call_value_internal define_expands, and
>> >simplifies the call and call_value define_expands.
>
> Is this patch really any better?  I can't see that it is myself.  It seems to
> me that the existing mechanism to emit the call, then append to FUNCTION_USAGE
> is perfectly clear.  This new argument to define_expand seems less clear.
>
> What are you trying to fix, anyway?

Richard,

In arm.md, the define_expand "call" rtl template is not used, because DONE is 
used in the preparation statements operand. The DONE is there, because we need a 
handle to the emitted insn to do post-emit processing.

The post-emission C-code operand that this patch introduces provides a handle to 
the emitted insn, which means we no longer need to use an explicit emit and DONE 
to get that handle.  And without that DONE, we can use the rtl template of 
define_expand "call", and no longer need the call_internal.

So we eliminate an define_expand (which is shorter, and removes duplicate code), 
and deal with expansion inside a single define_expand (which is clearer). To me, 
those are the benefits.

I agree the existing mechanism is perfectly clear.

Still I think that a 'finalization statements' operand is as clear as a 
'preparation statements' operand.

Thanks,
- Tom
diff mbox

Patch

diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 74645ee..506791a 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -126,7 +126,7 @@  extern int arm_const_double_inline_cost (rtx);
 extern bool arm_const_double_by_parts (rtx);
 extern bool arm_const_double_by_immediates (rtx);
 extern const char *fp_immediate_constant (rtx);
-extern void arm_emit_call_insn (rtx, rtx);
+extern void arm_post_emit_call_insn (rtx, rtx);
 extern const char *output_call (rtx *);
 extern const char *output_call_mem (rtx *);
 void arm_emit_movpair (rtx, rtx);
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 09b5c52..e36deac 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -17602,16 +17602,14 @@  vfp_emit_fstmd (int base_reg, int count)
   return count * 8;
 }
 
-/* Emit a call instruction with pattern PAT.  ADDR is the address of
-   the call target.  */
+/* Process a call instruction with pattern PAT after emission.  ADDR is the
+   address of the call target.  */
 
 void
-arm_emit_call_insn (rtx pat, rtx addr)
+arm_post_emit_call_insn (rtx pat, rtx addr)
 {
   rtx insn;
 
-  insn = emit_call_insn (pat);
-
   /* The PIC register is live on entry to VxWorks PIC PLT entries.
      If the call might use such an entry, add a use of the PIC register
      to the instruction's CALL_INSN_FUNCTION_USAGE.  */
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 8a949b9..45019ae 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -9082,7 +9082,7 @@ 
   "TARGET_EITHER"
   "
   {
-    rtx callee, pat;
+    rtx callee;
     
     /* In an untyped call, we can get NULL for operand 2.  */
     if (operands[2] == NULL_RTX)
@@ -9097,18 +9097,13 @@ 
 	: !REG_P (callee))
       XEXP (operands[0], 0) = force_reg (Pmode, callee);
 
-    pat = gen_call_internal (operands[0], operands[1], operands[2]);
-    arm_emit_call_insn (pat, XEXP (operands[0], 0));
-    DONE;
+  }"
+  []
+  "{
+    arm_post_emit_call_insn (_val, XEXP (operands[0], 0));
   }"
 )
 
-(define_expand "call_internal"
-  [(parallel [(call (match_operand 0 "memory_operand" "")
-	            (match_operand 1 "general_operand" ""))
-	      (use (match_operand 2 "" ""))
-	      (clobber (reg:SI LR_REGNUM))])])
-
 (define_insn "*call_reg_armv5"
   [(call (mem:SI (match_operand:SI 0 "s_register_operand" "r"))
          (match_operand 1 "" ""))
@@ -9191,7 +9186,7 @@ 
   "TARGET_EITHER"
   "
   {
-    rtx pat, callee;
+    rtx callee;
     
     /* In an untyped call, we can get NULL for operand 2.  */
     if (operands[3] == 0)
@@ -9205,21 +9200,13 @@ 
 	? arm_is_long_call_p (SYMBOL_REF_DECL (callee))
 	: !REG_P (callee))
       XEXP (operands[1], 0) = force_reg (Pmode, callee);
-
-    pat = gen_call_value_internal (operands[0], operands[1],
-				   operands[2], operands[3]);
-    arm_emit_call_insn (pat, XEXP (operands[1], 0));
-    DONE;
+  }"
+  []
+  "{
+    arm_post_emit_call_insn (_val, XEXP (operands[1], 0));
   }"
 )
 
-(define_expand "call_value_internal"
-  [(parallel [(set (match_operand       0 "" "")
-	           (call (match_operand 1 "memory_operand" "")
-		         (match_operand 2 "general_operand" "")))
-	      (use (match_operand 3 "" ""))
-	      (clobber (reg:SI LR_REGNUM))])])
-
 (define_insn "*call_value_reg_armv5"
   [(set (match_operand 0 "" "")
         (call (mem:SI (match_operand:SI 1 "s_register_operand" "r"))
diff --git a/gcc/genemit.c b/gcc/genemit.c
index faaa610..aff27f6 100644
--- a/gcc/genemit.c
+++ b/gcc/genemit.c
@@ -422,7 +422,8 @@  gen_expand (rtx expand)
   /* If we don't have any C code to write, only one insn is being written,
      and no MATCH_DUPs are present, we can just return the desired insn
      like we do for a DEFINE_INSN.  This saves memory.  */
-  if ((XSTR (expand, 3) == 0 || *XSTR (expand, 3) == '\0')
+  if (((XSTR (expand, 3) == 0 || *XSTR (expand, 3) == '\0'))
+      && (XSTR (expand, 5) == 0 || *XSTR (expand, 5) == '\0')
       && stats.max_opno >= stats.max_dup_opno
       && XVECLEN (expand, 1) == 1)
     {
@@ -525,6 +526,23 @@  gen_expand (rtx expand)
 
   printf ("  _val = get_insns ();\n");
   printf ("  end_sequence ();\n");
+
+  if (XSTR (expand, 5) && *XSTR (expand, 5))
+    {
+      printf ("  {\n");
+      if (stats.num_operand_vars > 0)
+	printf ("    rtx operands[%d];\n", stats.num_operand_vars);
+
+      /* Output code to copy the arguments into `operands'.  */
+      for (i = 0; i < stats.num_generator_args; i++)
+	printf ("    operands[%d] = operand%d;\n", i, i);
+
+      print_md_ptr_loc (XSTR (expand, 5));
+      printf ("%s\n", XSTR (expand, 5));
+
+      printf ("  }\n");
+    }
+
   printf ("  return _val;\n}\n\n");
 }
 
diff --git a/gcc/rtl.def b/gcc/rtl.def
index 56418c7..655f752 100644
--- a/gcc/rtl.def
+++ b/gcc/rtl.def
@@ -933,8 +933,10 @@  DEF_RTL_EXPR(DEFINE_PEEPHOLE2, "define_peephole2", "EsES", RTX_EXTRA)
 	elements of `recog_data.operand' for use by the vector of
 	insn-patterns.
 	(`operands' is an alias here for `recog_data.operand').
-   5th: optionally, a vector of attributes for this expand.  */
-DEF_RTL_EXPR(DEFINE_EXPAND, "define_expand", "sEssV", RTX_EXTRA)
+   5th: optionally, a vector of attributes for this expand.
+   6th operand: Extra C code to execute after generating the insns.  This code
+        is not executed if DONE (or FAIL) is used in the 4th operand.  */
+DEF_RTL_EXPR(DEFINE_EXPAND, "define_expand", "sEssVs", RTX_EXTRA)
 
 /* Define a requirement for delay slots.
    1st operand: Condition involving insn attributes that, if true,
-- 
1.8.3.2