[PATCH v2 02/16] RISC-V: fold redundant code in riscv_ip()

Jan Beulich jbeulich@suse.com
Fri May 15 13:29:18 GMT 2026


The parsing of 'F' and 'O' .insn operands is pretty redundant. Have only a
single instance each of common code, with the inner switch()es merely
handling the actual value insertion. This in particular simplifies the
addition of new sub-forms.
---
v2: Fix O2 to permit only values 0...2. Re-base over O4 -> O7 rename.
    Fold diagnostics in 'O' handling back into being a single one.

--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -3193,72 +3193,47 @@ riscv_ip (char *str, struct riscv_cl_ins
 		  INSERT_OPERAND (CRS2, *ip, regno);
 		  continue;
 		case 'F':
-		  switch (*++oparg)
+		  if (!ISDIGIT (*++oparg))
+		    goto unknown_riscv_ip_operand;
+
+		  /* (Ab)use "regno" here.  */
+		  regno = *oparg - '0';
+		  if (my_getSmallExpression (imm_expr, imm_reloc, asarg, p)
+		      || imm_expr->X_op != O_constant
+		      || imm_expr->X_add_number < 0
+		      || imm_expr->X_add_number >= (1U << regno))
+		    {
+		      as_bad (_("bad value for compressed funct%u "
+				"field, value must be 0...%u"),
+			      regno, (1U << regno) - 1);
+		      break;
+		    }
+
+		  switch (regno)
 		    {
-		      case '6':
-		        if (my_getSmallExpression (imm_expr, imm_reloc, asarg, p)
-			    || imm_expr->X_op != O_constant
-			    || imm_expr->X_add_number < 0
-			    || imm_expr->X_add_number >= 64)
-			  {
-			    as_bad (_("bad value for compressed funct6 "
-				      "field, value must be 0...63"));
-			    break;
-			  }
-			INSERT_OPERAND (CFUNCT6, *ip, imm_expr->X_add_number);
-			imm_expr->X_op = O_absent;
-			asarg = expr_parse_end;
-			continue;
-
-		      case '4':
-		        if (my_getSmallExpression (imm_expr, imm_reloc, asarg, p)
-			    || imm_expr->X_op != O_constant
-			    || imm_expr->X_add_number < 0
-			    || imm_expr->X_add_number >= 16)
-			  {
-			    as_bad (_("bad value for compressed funct4 "
-				      "field, value must be 0...15"));
-			    break;
-			  }
-			INSERT_OPERAND (CFUNCT4, *ip, imm_expr->X_add_number);
-			imm_expr->X_op = O_absent;
-			asarg = expr_parse_end;
-			continue;
-
-		      case '3':
-			if (my_getSmallExpression (imm_expr, imm_reloc, asarg, p)
-			    || imm_expr->X_op != O_constant
-			    || imm_expr->X_add_number < 0
-			    || imm_expr->X_add_number >= 8)
-			  {
-			    as_bad (_("bad value for compressed funct3 "
-				      "field, value must be 0...7"));
-			    break;
-			  }
-			INSERT_OPERAND (CFUNCT3, *ip, imm_expr->X_add_number);
-			imm_expr->X_op = O_absent;
-			asarg = expr_parse_end;
-			continue;
-
-		      case '2':
-			if (my_getSmallExpression (imm_expr, imm_reloc, asarg, p)
-			    || imm_expr->X_op != O_constant
-			    || imm_expr->X_add_number < 0
-			    || imm_expr->X_add_number >= 4)
-			  {
-			    as_bad (_("bad value for compressed funct2 "
-				      "field, value must be 0...3"));
-			    break;
-			  }
-			INSERT_OPERAND (CFUNCT2, *ip, imm_expr->X_add_number);
-			imm_expr->X_op = O_absent;
-			asarg = expr_parse_end;
-			continue;
+		    case 6:
+		      INSERT_OPERAND (CFUNCT6, *ip, imm_expr->X_add_number);
+		      break;
 
-		      default:
-			goto unknown_riscv_ip_operand;
+		    case 4:
+		      INSERT_OPERAND (CFUNCT4, *ip, imm_expr->X_add_number);
+		      break;
+
+		    case 3:
+		      INSERT_OPERAND (CFUNCT3, *ip, imm_expr->X_add_number);
+		      break;
+
+		    case 2:
+		      INSERT_OPERAND (CFUNCT2, *ip, imm_expr->X_add_number);
+		      break;
+
+		    default:
+		      goto unknown_riscv_ip_operand;
 		    }
-		  break;
+
+		  imm_expr->X_op = O_absent;
+		  asarg = expr_parse_end;
+		  continue;
 
 		default:
 		  goto unknown_riscv_ip_operand;
@@ -3709,97 +3684,80 @@ riscv_ip (char *str, struct riscv_cl_ins
 	      continue;
 
 	    case 'O':
-	      switch (*++oparg)
+	      if (!ISDIGIT (*++oparg))
+		goto unknown_riscv_ip_operand;
+
+	      /* (Ab)use "regno" here.  */
+	      regno = *oparg - '0';
+	      if (my_getOpcodeExpression (imm_expr, imm_reloc, asarg)
+		  || imm_expr->X_op != O_constant
+		  || imm_expr->X_add_number < 0
+		  /* O2 encodes C opcodes, i.e. doesn't permit value 3.  */
+		  || imm_expr->X_add_number >= (1U << regno) - (regno == 2)
+		  || (regno > 2 && (imm_expr->X_add_number & 3) != 3))
+		{
+		  as_bad (_("bad value for opcode field, "
+			    "value must be 0...%u%s"),
+			  (1U << regno) - (regno == 2) - 1,
+			  regno > 2 ? _(" and lower 2 bits must be 0x3") : "");
+		  break;
+		}
+
+	      switch (*oparg)
 		{
 		case '7':
-		  if (my_getOpcodeExpression (imm_expr, imm_reloc, asarg)
-		      || imm_expr->X_op != O_constant
-		      || imm_expr->X_add_number < 0
-		      || imm_expr->X_add_number >= 128
-		      || (imm_expr->X_add_number & 0x3) != 3)
-		    {
-		      as_bad (_("bad value for opcode field, "
-				"value must be 0...127 and "
-				"lower 2 bits must be 0x3"));
-		      break;
-		    }
 		  INSERT_OPERAND (OP, *ip, imm_expr->X_add_number);
-		  imm_expr->X_op = O_absent;
-		  asarg = expr_parse_end;
-		  continue;
+		  break;
 
 		case '2':
-		  if (my_getOpcodeExpression (imm_expr, imm_reloc, asarg)
-		      || imm_expr->X_op != O_constant
-		      || imm_expr->X_add_number < 0
-		      || imm_expr->X_add_number >= 3)
-		    {
-		      as_bad (_("bad value for opcode field, "
-				"value must be 0...2"));
-		      break;
-		    }
 		  INSERT_OPERAND (OP2, *ip, imm_expr->X_add_number);
-		  imm_expr->X_op = O_absent;
-		  asarg = expr_parse_end;
-		  continue;
+		  break;
 
 		default:
 		  goto unknown_riscv_ip_operand;
 		}
-	      break;
+
+	      imm_expr->X_op = O_absent;
+	      asarg = expr_parse_end;
+	      continue;
 
 	    case 'F':
-	      switch (*++oparg)
+	      if (!ISDIGIT (*++oparg))
+		goto unknown_riscv_ip_operand;
+
+	      /* (Ab)use "regno" here.  */
+	      regno = *oparg - '0';
+	      if (my_getSmallExpression (imm_expr, imm_reloc, asarg, p)
+		  || imm_expr->X_op != O_constant
+		  || imm_expr->X_add_number < 0
+		  || imm_expr->X_add_number >= (1U << regno))
 		{
-		case '7':
-		  if (my_getSmallExpression (imm_expr, imm_reloc, asarg, p)
-		      || imm_expr->X_op != O_constant
-		      || imm_expr->X_add_number < 0
-		      || imm_expr->X_add_number >= 128)
-		    {
-		      as_bad (_("bad value for funct7 field, "
-				"value must be 0...127"));
-		      break;
-		    }
+		  as_bad (_("bad value for funct%u field, "
+			    "value must be 0...%u"),
+			  regno, (1U << regno) - 1);
+		  break;
+		}
+	      switch (regno)
+		{
+		case 7:
 		  INSERT_OPERAND (FUNCT7, *ip, imm_expr->X_add_number);
-		  imm_expr->X_op = O_absent;
-		  asarg = expr_parse_end;
-		  continue;
+		  break;
 
-		case '3':
-		  if (my_getSmallExpression (imm_expr, imm_reloc, asarg, p)
-		      || imm_expr->X_op != O_constant
-		      || imm_expr->X_add_number < 0
-		      || imm_expr->X_add_number >= 8)
-		    {
-		      as_bad (_("bad value for funct3 field, "
-			        "value must be 0...7"));
-		      break;
-		    }
+		case 3:
 		  INSERT_OPERAND (FUNCT3, *ip, imm_expr->X_add_number);
-		  imm_expr->X_op = O_absent;
-		  asarg = expr_parse_end;
-		  continue;
+		  break;
 
-		case '2':
-		  if (my_getSmallExpression (imm_expr, imm_reloc, asarg, p)
-		      || imm_expr->X_op != O_constant
-		      || imm_expr->X_add_number < 0
-		      || imm_expr->X_add_number >= 4)
-		    {
-		      as_bad (_("bad value for funct2 field, "
-			        "value must be 0...3"));
-		      break;
-		    }
+		case 2:
 		  INSERT_OPERAND (FUNCT2, *ip, imm_expr->X_add_number);
-		  imm_expr->X_op = O_absent;
-		  asarg = expr_parse_end;
-		  continue;
+		  break;
 
 		default:
 		  goto unknown_riscv_ip_operand;
 		}
-	      break;
+
+	      imm_expr->X_op = O_absent;
+	      asarg = expr_parse_end;
+	      continue;
 
 	    case 'y': /* bs immediate */
 	      my_getExpression (imm_expr, asarg, force_reloc);



More information about the Binutils mailing list