Re: [PATCH] Merging Cilk Plus into Trunk (Patch 1 of approximately 22)

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Wed, 5 Sep 2012, Iyer, Balaji V wrote:

> 	Attached, please find the 1st of ~22 patches that implements Cilk 
> Plus. This patch will implement Elemental Functions into the C compiler.  
> Please check it in to the trunk if it looks OK.
> 
> 	Below, I will give you a small example about what elemental 
> function is and how it can be useful. Details about elemental function 
> can be found in the following link 
> (http://software.intel.com/en-us/articles/elemental-functions-writing-data-parallel-code-in-cc-using-intel-cilk-plus)

That page says "To continue reading the article, click on the link below." 
but I don't see such a link below.

>         * c-cpp-elem-function.c (create_processor_attribute): Likewise.

I don't see a ChangeLog entry for the addition of this file at all.  When 
a new file is added, "New file." is enough entry; you don't describe 
particular things within the file.

This file includes tm.h and tm_p.h.  Inclusion of these headers from 
front-end code is deprecated.  If they are really needed, please put 
comments on the includes about exactly what target macros are being used 
in this front-end code.  Similarly, use of hard-reg-set.h in front-end 
code is doubtful.  Generally, please check all #includes in all new source 
files and make sure that each include is actually needed because some 
functionality from the relevant header is used in the source file; do not 
just copy the headers included by some existing source file.

create_processor_attribute contains hardcoded references to x86-specific 
functionality.  This is not OK; all such target dependencies need to be 
kept within the back ends, and handled from the rest of the compiler via 
target hooks (in most cases, new target dependencies must use target hooks 
not target macros).

Please make sure every new function has a comment explicitly describing 
the semantics of every parameter and the return value as well as anything 
else the function does.

Where there are alternative versions of functions/macros with/without 
explicit locations, please use the forms with explicit locations (e.g. 
build2_loc instead of build2), and try to link the locations to particular 
source code tokens and pass those locations down explicitly to each 
function as needed.

There may be more issues; I'll await a revised patch before doing further 
review.

-- 
Joseph S. Myers
joseph@codesourcery.com