Re: [off-list]: "enabled" attribute patch

["Andreas Krebbel" <Andreas dot Krebbel at de dot ibm dot com>]

Hi,

> Also, the new extract_insn calls alternative_enabled_p_alt
> before setting up the recog_data.operand[] array.  I think
> that's too early, because attributes can use match_operand.
> Perhaps the safest place would be to do it at the end?

Agreed.

> I've assumed Mark's suggested change
> of removing the preprocessor conditions.  (I think the easiest way
> of doing that would be to provide a default inline get_enabled_attr
> function that always returns true.)

Good idea.

> A minor niggle, but I notice you've got three copies of:
> 
> 		while (*p != '\0' && *p != ',')
> 		  p++;
> 		if (*p == ',')
> 		  p++;
> 
> How about adding a new (inline?) function for getting to
> the next alternative's constraint?

Ok.

> (I was reluctant to say anything because, after Andreas responded
> to a comment of mine on the TARGET_MEM_ATTRIBUTE patch, it too has
> gone unreviewed after many pings, even though it's much simpler
> than this one.  Hope I'm not cursing him here.)

Hey - no sweat!  The delay obviously wasn't your fault.  Your comments
have always been helpful. So please don't hesitate next time.

> +      recog_data.insn = insn;
> +      for (i = 0; i < recog_data.n_alternatives; i++)
> +	{
> +	  which_alternative = i;
> +	  recog_data.alternative_enabled_p[i] = get_enabled_attr (insn);
> +	}

Looks like a wonderful hack to me.  I don't see a reason why this
shouldn't work and no reason why I never tried it that way before
wading through genattr.c :( I've started a bootstrap together with
the s390 cpu_facility patch to see whether that works out.  If we
could that way get rid of the *_alt accessor functions it would be
really nice. But I'm not sure whether this abuse of the interface is
acceptable for GCC.  Isn't that the kind of hacks which come back and
bite you some day?

Bye,

-Andreas-