This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, asmcons] Fix PR rtl-optimization/89313: ICE in process_alt_operands, at lra-constraints.c:2962


Hi Peter,

On Wed, Feb 20, 2019 at 09:19:58PM -0600, Peter Bergner wrote:
> PR89313 exposes a bug in the asmcons pass where it replaces input operands
> with matching constraints with their associated output operands, as well as
> all other uses of its pseudo registers.  This is normally fine.  However, if
> the matched output operand is marked as early clobber, then we cannot replace
> the uses of 'input' that do not have matching constraints, since they by
> definition conflict with the early clobber output operand and could be
> clobbered if assigned to the same register as the output operand.
> 
> The patch below fixes the bug by only doing the input pseudo replacement
> if the output operand is not early clobber or the input operand is known
> to be a matching constraint.
> 
> This passed bootstrap and regression testing with no regressions on
> both x86_64-linux and powerpc64le-linux.  Ok for mainline?

Looks fine to me, but I cannot approve it of course.  Some trivial
comments:

> +/* If CONSTRAINT is a matching constraint, then return its number.
> +   Otherwise, return -1.  */
> +
> +static int
> +matching_constraint_num (const char *constraint)
> +{
> +  int match;
> +
> +  if (*constraint == '%')
> +    constraint++;
> +
> +  switch (*constraint)
> +    {
> +    case '0': case '1': case '2': case '3': case '4':
> +    case '5': case '6': case '7': case '8': case '9':
> +      {
> +	char *end;
> +	match = strtoul (constraint, &end, 10);
> +	if (end == constraint)
> +	  match = -1;

This condition is always false, because we have at least one digit.

> +	break;
> +      }
> +
> +    default:
> +      match = -1;
> +      break;
> +    }
> +  return match;
> +}

Which means you can write it as just

static int
matching_constraint_num (const char *constraint)
{
  if (*constraint == '%')
    constraint++;

  if (IN_RANGE (*constraint, '0', '9'))
    return strtoul (constraint, NULL, 10);

  return -1;
}


Segher


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]