Patchwork C6X port 9/11: Allow defining attributes in terms of another

login
register
mail settings
Submitter Bernd Schmidt
Date May 10, 2011, 3:47 p.m.
Message ID <4DC95DFE.5050902@codesourcery.com>
Download mbox | patch
Permalink /patch/95061/
State New
Headers show

Comments

Bernd Schmidt - May 10, 2011, 3:47 p.m.
I've found it useful to use a construct such as the following:

(define_attr "units64"
  "unknown,d,d_addr,l,m,s,dl,ds,dls,ls"
  (const_string "unknown"))

(define_attr "units64p"
  "unknown,d,d_addr,l,m,s,dl,ds,dls,ls"
  (attr "units64"))

to define one attribute in terms of another by default, allowing
individual insn patterns to override the definition of "units64p" where
necessary. This patch adds support for this in genattrtab.


Bernd
* genattrtab.c (evaluate_eq_attr): Allow an attribute to be defined
	in terms of another.
Hans-Peter Nilsson - May 25, 2011, 8:56 a.m.
On Tue, 10 May 2011, Bernd Schmidt wrote:

> I've found it useful to use a construct such as the following:
>
> (define_attr "units64"
>   "unknown,d,d_addr,l,m,s,dl,ds,dls,ls"
>   (const_string "unknown"))
>
> (define_attr "units64p"
>   "unknown,d,d_addr,l,m,s,dl,ds,dls,ls"
>   (attr "units64"))
>
> to define one attribute in terms of another by default,

So it's just the units64p default value taken from the units64
default value or units64p gets its default value from the final
units64 value?

> allowing
> individual insn patterns to override the definition of "units64p" where
> necessary. This patch adds support for this in genattrtab.

I'm not sure I get it, and I think I would be helped by seeing
the documentation update. ;)

brgds, H-P
PS. ok, I could look in the port, but the documentation still needs updating, right?
Bernd Schmidt - May 25, 2011, 9:58 a.m.
On 05/25/2011 08:56 AM, Hans-Peter Nilsson wrote:
> On Tue, 10 May 2011, Bernd Schmidt wrote:
> 
>> I've found it useful to use a construct such as the following:
>>
>> (define_attr "units64"
>>   "unknown,d,d_addr,l,m,s,dl,ds,dls,ls"
>>   (const_string "unknown"))
>>
>> (define_attr "units64p"
>>   "unknown,d,d_addr,l,m,s,dl,ds,dls,ls"
>>   (attr "units64"))
>>
>> to define one attribute in terms of another by default,
> 
> So it's just the units64p default value taken from the units64
> default value or units64p gets its default value from the final
> units64 value?

units64p has the final value of units64, unless an insn explicitly gives
it a different value. This is because C64X+ is really very similar to
C64X in most respects. We then select which of the various units
definitions to use for a given CPU:

(define_attr "units"
  "unknown,d,d_addr,l,m,s,dl,ds,dls,ls"
  (cond [(eq_attr "cpu" "c62x")
	   (attr "units62")
	 (eq_attr "cpu" "c67x")
	   (attr "units67")
	 (eq_attr "cpu" "c67xp")
	   (attr "units67p")
	 (eq_attr "cpu" "c64x")
	   (attr "units64")
	 (eq_attr "cpu" "c64xp")
	   (attr "units64p")
	 (eq_attr "cpu" "c674x")
	   (attr "units674")
	]
	(const_string "unknown")))

>> allowing
>> individual insn patterns to override the definition of "units64p" where
>> necessary. This patch adds support for this in genattrtab.
> 
> I'm not sure I get it, and I think I would be helped by seeing
> the documentation update. ;)

I'm not sure where you're looking for added documentation for this
patch. It just generalizes the define_attr mechanism a little to allow
one more kind of expression.


Bernd
Hans-Peter Nilsson - May 25, 2011, 1:45 p.m.
On Wed, 25 May 2011, Bernd Schmidt wrote:

> I'm not sure where you're looking for added documentation for this
> patch.

I guess no surprise that'd be md.texi node Defining Attributes,
or an updated example in node Attr Example since the
documentation for "default" basically just refers to it.  Or
perhaps better node Expressions where (attr x) is documented,
since it says it's mostly useful for numeric attributes and not
so for non-numeric attributes.  Perhaps add after that sentence
"It can also be used to yield the value of another attribute,
useful to e.g. set the value of the current attribute if they
share a domain".  You can probably find a better wording. :)

> It just generalizes the define_attr mechanism a little to
> allow one more kind of expression.

Yes, the documentation is a bit terse, isn't it.  But the idea
that you can redirect to another attribute instead of referring
to it in a conditional like in eq_attr seems to me new enough to
warrant a line.

brgds, H-P
Bernd Schmidt - May 26, 2011, 1:31 p.m.
On 05/25/2011 01:45 PM, Hans-Peter Nilsson wrote:
> On Wed, 25 May 2011, Bernd Schmidt wrote:
> 
>> I'm not sure where you're looking for added documentation for this
>> patch.
> 
> I guess no surprise that'd be md.texi node Defining Attributes,

That covers define_attr, not set_attr, so it seems inappropriate.

> or an updated example in node Attr Example since the
> documentation for "default" basically just refers to it.  Or
> perhaps better node Expressions where (attr x) is documented,
> since it says it's mostly useful for numeric attributes and not
> so for non-numeric attributes.  Perhaps add after that sentence
> "It can also be used to yield the value of another attribute,
> useful to e.g. set the value of the current attribute if they
> share a domain".  You can probably find a better wording. :)

To be honest I can't think of anything reasonable to add there. The
documentation already says you can use (attr "...") to return the value
of another attribute... all my patch did was make that actually work in
some situations.


Bernd

Patch

Index: genattrtab.c
===================================================================
--- genattrtab.c.orig
+++ genattrtab.c
@@ -1916,6 +1916,37 @@  evaluate_eq_attr (rtx exp, struct attr_d
   rtx newexp;
   int i;
 
+  while (GET_CODE (value) == ATTR)
+    {
+      struct attr_value *av = NULL;
+
+      attr = find_attr (&XSTR (value, 0), 0);
+
+      if (insn_code_values)
+        {
+          struct attr_value_list *iv;
+          for (iv = insn_code_values[insn_code]; iv; iv = iv->next)
+            if (iv->attr == attr)
+              {
+                av = iv->av;
+                break;
+              }
+        }
+      else
+        {
+          struct insn_ent *ie;
+          for (av = attr->first_value; av; av = av->next)
+            for (ie = av->first_insn; ie; ie = ie->next)
+              if (ie->def->insn_code == insn_code)
+                goto got_av;
+        }
+      if (av)
+        {
+        got_av:
+          value = av->value;
+        }
+    }
+
   switch (GET_CODE (value))
     {
     case CONST_STRING: