[off-list]: "enabled" attribute patch

[Mark Mitchell]

Uros Bizjak wrote:
> On Tue, Apr 29, 2008 at 12:16 AM, Mark Mitchell <mark@codesourcery.com> wrote:
> 
>> Please point me at the URL. Thanks,
> 
> Generic part of the patch is here:
> 
> http://gcc.gnu.org/ml/gcc-patches/2008-04/msg01200.html
> http://gcc.gnu.org/ml/gcc-patches/2008-04/msg01201.html

[Uros sent me private email asking me to review the "enabled" attribute 
patch.  I'm sending this reply back to the list.]

I've looked at the patch.  I like the idea.

However, it needs documentation.  As a non-expert on machine 
descriptions, it took me a while to work out what was going on, but I 
think I mostly understand it now.

In particular:

1. The cpu_facility attribute is defined as an enumeration of capabilities.

2. For each instruction, you can define a cpu_facility value for each 
alternative.  A given alternative is only available if the CPU actually 
in use has the facility given.

However, I don't understand how "enabled" ties that into the generated 
instruction tables.

Also, what if an alternative requires multiple CPU facilities?  For 
example, it works only on a CPU that has both the optional multiply-add 
unit and the optional divide unit?  Can you do something like 
"muladd&div" in the alternative list?  If not, should we add that facility?

As a minor stylistic comment, I think we should avoid the #ifdef 
HAVE_ATTR_enabled conditionals.  Our general policy is to use "if", not 
"#ifdef".  So, use:

#ifndef HAVE_ATTR_ENABLED
#define HAVE_ATTR_ENABLED 0
#endif
...
if (HAVE_ATTR_enabled)

instead of:

#ifdef HAVE_ATTR_enabled

I realize this will mean that we have the alternative_enabled_p array on 
all machines, but I'd be surprised if we can measure that cost.

Do any of the folks with more knowledge of back-ends have any comments 
on this patch?  Otherwise, it's my intention to work with Uros and 
Andreas to get this patch merged in.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713