On 5/29/23 06:46, Jeff Law wrote:


On 5/29/23 05:01, Jin Ma wrote:
Reference: https://github.com/gcc-mirror/gcc/commit/d0bc0cb66bcb0e6a5a5a31a9e900e8ccc98e34e5

RISC-V should also be implemented to handle no_insn patterns for pipelining.

gcc/ChangeLog:

    * config/riscv/riscv.cc (riscv_sched_variable_issue): New function.
    (TARGET_SCHED_VARIABLE_ISSUE): New macro.
---
  gcc/config/riscv/riscv.cc | 21 +++++++++++++++++++++
  1 file changed, 21 insertions(+)

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 3954fc07a8b..559fa9cd7e0 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -6225,6 +6225,24 @@ riscv_issue_rate (void)
    return tune_param->issue_rate;
  }
+/* Implement TARGET_SCHED_VARIABLE_ISSUE.  */
+
+static int
+riscv_sched_variable_issue (FILE *, int, rtx_insn *insn, int more)
+{
+  if (DEBUG_INSN_P (insn))
+    return more;
+
+  rtx_code code = GET_CODE (PATTERN (insn));
+  if (code == USE || code == CLOBBER)
+    return more;
+
+  if (get_attr_type (insn) == TYPE_UNKNOWN)
+    return more;
+
+  return more - 1;
+}
The problem is that INSN is *much* more likely to be a real instruction that takes real resources, even if it is TYPE_UNKNOWN. TYPE_UNKNOWN here is actually an indicator of what I would consider a bug in the backend, specifically that we have INSNs that do not provide a mapping for the schedulers to suitable types.

With that in mind I'd much rather get to the point where we can do something like this for TYPE_UNKNOWN:

type = get_attr_type (insn);
gcc_assert (type != TYPE_UNKNOWN);

That way if we ever encounter a TYPE_UNKNOWN during development, we can fix it in the md files in a sensible manner.  I don't know if we are close to being able to do that.  We fixed a ton of stuff in bitmanip.md, but I don't think there's been a thorough review of the port to find other instances of TYPE_UNKNOWN INSNs.


The other thing if this code probably wants to handle GHOST type instructions.  While GHOST is used for instructions which generate no code, it might seem they should return "more" as those INSNs take no resources.  But GHOST is actually used for things like the blockage insn which should end a cycle from an issue standpoint.  So the right handling of a GHOST is something like this:

if (type == TYPE_GHOST)
   return 0;
So there wasn't ever any follow-up. Given this was something Ventana was also carrying locally (with very minor differences) I went ahead and merged up the implementations and pushed the final result to the trunk.


Attached is the patch that was actually committed.

Jeff
commit f088b768d01ae42385697584a2bcac141685dce2
Author: Jin Ma <ji...@linux.alibaba.com>
Date:   Wed Aug 9 13:52:06 2023 -0600

    RISC-V: Handle no_insn in TARGET_SCHED_VARIABLE_ISSUE.
    
    Reference: 
https://github.com/gcc-mirror/gcc/commit/d0bc0cb66bcb0e6a5a5a31a9e900e8ccc98e34e5
    
    RISC-V should also be implemented to handle no_insn patterns for pipelining.
    
    gcc/ChangeLog:
    
            * config/riscv/riscv.cc (riscv_sched_variable_issue): New function.
            (TARGET_SCHED_VARIABLE_ISSUE): New macro.
    
            Co-authored-by: Philipp Tomsich <philipp.toms...@vrull.eu>
            Co-authored-by: Jeff Law <j...@ventanamicro.com>

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 7f2041a54ba..dfb519ab9a8 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -6698,6 +6698,31 @@ riscv_issue_rate (void)
   return tune_param->issue_rate;
 }
 
+/* Implement TARGET_SCHED_VARIABLE_ISSUE.  */
+static int
+riscv_sched_variable_issue (FILE *, int, rtx_insn *insn, int more)
+{
+  if (DEBUG_INSN_P (insn))
+    return more;
+
+  rtx_code code = GET_CODE (PATTERN (insn));
+  if (code == USE || code == CLOBBER)
+    return more;
+
+  /* GHOST insns are used for blockage and similar cases which
+     effectively end a cycle.  */
+  if (get_attr_type (insn) == TYPE_GHOST)
+    return 0;
+
+#if 0
+  /* If we ever encounter an insn with an unknown type, trip
+     an assert so we can find and fix this problem.  */
+  gcc_assert (get_attr_type (insn) != TYPE_UNKNOWN);
+#endif
+
+  return more - 1;
+}
+
 /* Auxiliary function to emit RISC-V ELF attribute. */
 static void
 riscv_emit_attribute ()
@@ -8420,6 +8445,9 @@ riscv_frame_pointer_required (void)
 #undef TARGET_SCHED_ISSUE_RATE
 #define TARGET_SCHED_ISSUE_RATE riscv_issue_rate
 
+#undef  TARGET_SCHED_VARIABLE_ISSUE
+#define TARGET_SCHED_VARIABLE_ISSUE riscv_sched_variable_issue
+
 #undef TARGET_FUNCTION_OK_FOR_SIBCALL
 #define TARGET_FUNCTION_OK_FOR_SIBCALL riscv_function_ok_for_sibcall
 

Reply via email to