Re: [google][4.6]Compiler Directed Multiversioning with new -mvarch option (issue 5490054)

2011-12-17 Thread Xinliang David Li
ok for google branches.

David

On Fri, Dec 16, 2011 at 2:05 PM,  tmsri...@google.com wrote:
 I have uploaded a new patch set with all the mentioned changes made. If
 a function has the target attribute it will not be touched by the
 autoclone pass.

 Also fixed some test cases which were broken because the clone names
 used '_' instead of '.' for suffixing.


 On 2011/12/16 19:39:47, davidxl wrote:

 http://codereview.appspot.com/5490054/diff/1011/config/i386/i386.c
 File config/i386/i386.c (right):



 http://codereview.appspot.com/5490054/diff/1011/config/i386/i386.c#newcode26569

 config/i386/i386.c:26569: +mversion_for_core2 (tree

 *optimization_node,

 - mversionable_for_core2_p ?


 http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c
 File mversn-dispatch.c (right):



 http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode931

 mversn-dispatch.c:931: DECL_STATIC_DESTRUCTOR (new_decl) = 0;
 Should you assert it instead? Should not clone ctor/dtors.



 http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode2221

 mversn-dispatch.c:2221: VEC_truncate (edge, EXIT_BLOCK_PTR-preds, 0);
 {} -- remove



 http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode2389

 mversn-dispatch.c:2389:
 How does it interact with manual multi-versioning from user? You

 probably don't

 want to clone functions that are marked with target attributes

 (explicitly --

 not implied from command line).




 http://codereview.appspot.com/5490054/


Re: [google][4.6]Compiler Directed Multiversioning with new -mvarch option (issue 5490054)

2011-12-17 Thread Sriraman Tallam
Committed to google 4_6 branch.

Thanks,
-Sri.

On Sat, Dec 17, 2011 at 12:25 AM, Xinliang David Li davi...@google.com wrote:
 ok for google branches.

 David

 On Fri, Dec 16, 2011 at 2:05 PM,  tmsri...@google.com wrote:
 I have uploaded a new patch set with all the mentioned changes made. If
 a function has the target attribute it will not be touched by the
 autoclone pass.

 Also fixed some test cases which were broken because the clone names
 used '_' instead of '.' for suffixing.


 On 2011/12/16 19:39:47, davidxl wrote:

 http://codereview.appspot.com/5490054/diff/1011/config/i386/i386.c
 File config/i386/i386.c (right):



 http://codereview.appspot.com/5490054/diff/1011/config/i386/i386.c#newcode26569

 config/i386/i386.c:26569: +mversion_for_core2 (tree

 *optimization_node,

 - mversionable_for_core2_p ?


 http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c
 File mversn-dispatch.c (right):



 http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode931

 mversn-dispatch.c:931: DECL_STATIC_DESTRUCTOR (new_decl) = 0;
 Should you assert it instead? Should not clone ctor/dtors.



 http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode2221

 mversn-dispatch.c:2221: VEC_truncate (edge, EXIT_BLOCK_PTR-preds, 0);
 {} -- remove



 http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode2389

 mversn-dispatch.c:2389:
 How does it interact with manual multi-versioning from user? You

 probably don't

 want to clone functions that are marked with target attributes

 (explicitly --

 not implied from command line).




 http://codereview.appspot.com/5490054/


Re: [google][4.6]Compiler Directed Multiversioning with new -mvarch option (issue 5490054)

2011-12-16 Thread davidxl


http://codereview.appspot.com/5490054/diff/1011/config/i386/i386.c
File config/i386/i386.c (right):

http://codereview.appspot.com/5490054/diff/1011/config/i386/i386.c#newcode26569
config/i386/i386.c:26569: +mversion_for_core2 (tree *optimization_node,
- mversionable_for_core2_p ?

http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c
File mversn-dispatch.c (right):

http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode931
mversn-dispatch.c:931: DECL_STATIC_DESTRUCTOR (new_decl) = 0;
Should you assert it instead? Should not clone ctor/dtors.

http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode2221
mversn-dispatch.c:2221: VEC_truncate (edge, EXIT_BLOCK_PTR-preds, 0);
{} -- remove

http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode2389
mversn-dispatch.c:2389:
How does it interact with manual multi-versioning from user? You
probably don't want to clone functions that are marked with target
attributes (explicitly -- not implied from command line).

http://codereview.appspot.com/5490054/


Re: [google][4.6]Compiler Directed Multiversioning with new -mvarch option (issue 5490054)

2011-12-16 Thread tmsriram

I have uploaded a new patch set with all the mentioned changes made. If
a function has the target attribute it will not be touched by the
autoclone pass.

Also fixed some test cases which were broken because the clone names
used '_' instead of '.' for suffixing.

On 2011/12/16 19:39:47, davidxl wrote:

http://codereview.appspot.com/5490054/diff/1011/config/i386/i386.c
File config/i386/i386.c (right):



http://codereview.appspot.com/5490054/diff/1011/config/i386/i386.c#newcode26569

config/i386/i386.c:26569: +mversion_for_core2 (tree

*optimization_node,

- mversionable_for_core2_p ?



http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c
File mversn-dispatch.c (right):



http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode931

mversn-dispatch.c:931: DECL_STATIC_DESTRUCTOR (new_decl) = 0;
Should you assert it instead? Should not clone ctor/dtors.



http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode2221

mversn-dispatch.c:2221: VEC_truncate (edge, EXIT_BLOCK_PTR-preds, 0);
{} -- remove



http://codereview.appspot.com/5490054/diff/1011/mversn-dispatch.c#newcode2389

mversn-dispatch.c:2389:
How does it interact with manual multi-versioning from user? You

probably don't

want to clone functions that are marked with target attributes

(explicitly --

not implied from command line).




http://codereview.appspot.com/5490054/