From: Thierry L. <thi...@in...> - 2010-03-08 16:39:47
|
Index: gcc/config/cil32/cil-lower.c =================================================================== --- gcc/config/cil32/cil-lower.c (revision 156016) +++ gcc/config/cil32/cil-lower.c (working copy) @@ -1,6 +1,6 @@ /* CIL IR lowering. - Copyright (C) 2009 Free Software Foundation, Inc. + Copyright (C) 2009-2010 Free Software Foundation, Inc. This file is part of GCC. @@ -23,11 +23,13 @@ Andrea C. Ornstein Erven Rohou Gabriele Svelto + Thierry Lafage Contact information at STMicroelectronics: Andrea C. Ornstein <and...@st...> Contact information at INRIA: Erven Rohou <erv...@in...> +Thierry Lafage <thi...@in...> */ #include <stdio.h> @@ -52,6 +54,8 @@ * Globals * ******************************************************************************/ +extern int flag_tree_vectorize; // TL // This avoids including options.h + enum simd_backend simd_type = UNDEF_SIMD; /****************************************************************************** @@ -591,8 +595,7 @@ static bool lower_cil_gate (void) { - /* FIXME: should lower only if vector types are handled in this funtion. */ - return true; + return flag_tree_vectorize != 0; } |
From: Gabriele S. <gab...@gm...> - 2010-03-08 19:57:25
|
Hi Thierry. 2010/3/8 Thierry Lafage <thi...@in...>: > Hi all, > > Here are several small patches for our gcc4cli BE branch: > > pr19340.c.patch is to avoid testing with pr19340.c because it has no sense > in cil32 since it tests the scheduler. > gcc4cli_line_info-t-cil32.patch fixes dependencies in t-cil32 and adds the > file source-location.h which was introduced in my last patch concerning > source location (this t-cil32 patch should have been part of the source > location patch). > cil-lower.c.patch modifies the pass gate in order to activate this pass only > when -ftree_vectorize option is activated (which is the case in -O3) Unfortunately you cannot do this as the users can explicitly use vector intrinsics in the code even if -O3 is not specified. Under those conditions you will find vector operations even if you are compiling with -O0, it happens quite often in the testsuite BTW. > and simp-cond.patch is a simplification of the simp-cond pass which also > makes it more complete (adds support for brtrue/brfalse and for floating > point comparisons in conditional branches + add a command-line option which > is activated in -O1 + fixes missing brtrue/brfalse as being conditional > branch instructions). Note that this pass either does nothing, or change a > branch condition instruction into another one (inverts the condition) and > removes the following unconditional branch. This means that the stack is not > changed at all, so I removed all the stuff that was recomputing the stack at > every statement. Moreover, I didn't understand the test on the conditional > branch operand type (integer or pointer) in order to change the conditional > br instruction: I've removed it and tested on a real program, and it seems > to be ok. You cannot remove the integer / pointer test from that pass because for floating-point numbers those operations cannot be changed. For example if 'x' and 'y' are two floating-point NaNs the result from any comparison will always be unordered, i.e. x > y and x <= y will both evaluate to false hence you cannot invert the comparison. That's why I left out floating-point comparisons and that's why I compute the stack: I need to know on what types the comparison is working on. As for brtrue and brfalse I don't remember why I have left them out. They are different from the other instructions in that they pop off the stack a single value and work only on native integers while other comparisons pop two values which can be pretty much anything. I might have left them out because of that but I don't remember exactly so you might give them a try anyway. A full testsuite will give you the answer anyway, if there are problems tied to swapping brtrue/brfalse they will probably manifest themselves there. Gabriele |
From: Thierry L. <thi...@in...> - 2010-03-09 10:38:15
|
Hi Gabriele, Thanks for your quick answer. My remarks below... Gabriele Svelto a écrit : > Hi Thierry. > > 2010/3/8 Thierry Lafage <thi...@in...>: > >> cil-lower.c.patch modifies the pass gate in order to activate this pass only >> when -ftree_vectorize option is activated (which is the case in -O3) >> > Unfortunately you cannot do this as the users can explicitly use > vector intrinsics in the code even if -O3 is not specified. Under > those conditions you will find vector operations even if you are > compiling with -O0, it happens quite often in the testsuite BTW. > Ok, sorry, I misunderstood the fixme "should lower only if vector types are handled in this funtion." I simply wanted, for easier debug, to ensure that the execution of this pass could be controlled (ie. skipped by using a command-line argument). But it's probably useless. >> and simp-cond.patch is a simplification of the simp-cond pass which also >> makes it more complete (adds support for brtrue/brfalse and for floating >> point comparisons in conditional branches + add a command-line option which >> is activated in -O1 + fixes missing brtrue/brfalse as being conditional >> branch instructions). Note that this pass either does nothing, or change a >> branch condition instruction into another one (inverts the condition) and >> removes the following unconditional branch. This means that the stack is not >> changed at all, so I removed all the stuff that was recomputing the stack at >> every statement. Moreover, I didn't understand the test on the conditional >> branch operand type (integer or pointer) in order to change the conditional >> br instruction: I've removed it and tested on a real program, and it seems >> to be ok. >> > > You cannot remove the integer / pointer test from that pass because > for floating-point numbers those operations cannot be changed. For > example if 'x' and 'y' are two floating-point NaNs the result from any > comparison will always be unordered, i.e. x > y and x <= y will both > evaluate to false hence you cannot invert the comparison. That's why I > left out floating-point comparisons and that's why I compute the > stack: I need to know on what types the comparison is working on. As > for brtrue and brfalse I don't remember why I have left them out. They > are different from the other instructions in that they pop off the > stack a single value and work only on native integers while other > comparisons pop two values which can be pretty much anything. I might > have left them out because of that but I don't remember exactly so you > might give them a try anyway. A full testsuite will give you the > answer anyway, if there are problems tied to swapping brtrue/brfalse > they will probably manifest themselves there. > > Gabriele > Ok, for this one, I've been very naive. Sorry. It seems to me that it has been a fast copy/paste of a generic pass... For true/false, it happens that conditions on floating point values may be computed by first using a comparison, and then a brtrue or brfalse. So inverting the branch may lead to the same problems as with branches with FP operands. In this particular case, we could look up at the operands of the compare (if any) and invert the branch (or better: the compare itself?) only if they are integer or pointers. For the FP general case, wouldn't it be possible to use *.un (or not) versions of the branches? I'll make tests to check. However, concerning stack computation, since conditional branch inversion does not change the stack at all, are the "cil_stack_after_stmt (stack, stmt);" and "FOR_EACH_EDGE(...) { cil_set_stack_for_bb (...); }" useful? I understand these statements as being a stack re-compute, isn't it? Regards, Thierry. |
From: Erven R. <erv...@in...> - 2010-03-09 10:53:52
|
Thierry Lafage a écrit : > Hi Gabriele, > > Thanks for your quick answer. My remarks below... > > Gabriele Svelto a écrit : >> Hi Thierry. >> >> 2010/3/8 Thierry Lafage <thi...@in...>: >> >>> cil-lower.c.patch modifies the pass gate in order to activate this pass only >>> when -ftree_vectorize option is activated (which is the case in -O3) >>> >> Unfortunately you cannot do this as the users can explicitly use >> vector intrinsics in the code even if -O3 is not specified. Under >> those conditions you will find vector operations even if you are >> compiling with -O0, it happens quite often in the testsuite BTW. >> > Ok, sorry, I misunderstood the fixme "should lower only if vector types > are handled in this funtion." > I simply wanted, for easier debug, to ensure that the execution of this > pass could be controlled (ie. skipped by using a command-line argument). > But it's probably useless. I made an attempt along those lines some time ago. My approach was to detect whether a function "deals with" vectors, for some definition of "deals with". You can grep "has_vec". The idea was to run the pass only is has_vec is true. Unfortunately I was missing some cases. It is still not working. -- Erven. > >>> and simp-cond.patch is a simplification of the simp-cond pass which also >>> makes it more complete (adds support for brtrue/brfalse and for floating >>> point comparisons in conditional branches + add a command-line option which >>> is activated in -O1 + fixes missing brtrue/brfalse as being conditional >>> branch instructions). Note that this pass either does nothing, or change a >>> branch condition instruction into another one (inverts the condition) and >>> removes the following unconditional branch. This means that the stack is not >>> changed at all, so I removed all the stuff that was recomputing the stack at >>> every statement. Moreover, I didn't understand the test on the conditional >>> branch operand type (integer or pointer) in order to change the conditional >>> br instruction: I've removed it and tested on a real program, and it seems >>> to be ok. >>> >> You cannot remove the integer / pointer test from that pass because >> for floating-point numbers those operations cannot be changed. For >> example if 'x' and 'y' are two floating-point NaNs the result from any >> comparison will always be unordered, i.e. x > y and x <= y will both >> evaluate to false hence you cannot invert the comparison. That's why I >> left out floating-point comparisons and that's why I compute the >> stack: I need to know on what types the comparison is working on. As >> for brtrue and brfalse I don't remember why I have left them out. They >> are different from the other instructions in that they pop off the >> stack a single value and work only on native integers while other >> comparisons pop two values which can be pretty much anything. I might >> have left them out because of that but I don't remember exactly so you >> might give them a try anyway. A full testsuite will give you the >> answer anyway, if there are problems tied to swapping brtrue/brfalse >> they will probably manifest themselves there. >> >> Gabriele >> > > Ok, for this one, I've been very naive. Sorry. It seems to me that it > has been a fast copy/paste of a generic pass... > For true/false, it happens that conditions on floating point values may > be computed by first using a comparison, and then a brtrue or brfalse. > So inverting the branch may lead to the same problems as with branches > with FP operands. In this particular case, we could look up at the > operands of the compare (if any) and invert the branch (or better: the > compare itself?) only if they are integer or pointers. > For the FP general case, wouldn't it be possible to use *.un (or not) > versions of the branches? I'll make tests to check. > However, concerning stack computation, since conditional branch > inversion does not change the stack at all, are the > "cil_stack_after_stmt (stack, stmt);" and "FOR_EACH_EDGE(...) { > cil_set_stack_for_bb (...); }" useful? > I understand these statements as being a stack re-compute, isn't it? > > Regards, > Thierry. > > > > ------------------------------------------------------------------------------ > Download Intel® Parallel Studio Eval > Try the new software tools for yourself. Speed compiling, find bugs > proactively, and fine-tune applications for parallel performance. > See why Intel Parallel Studio got high marks during beta. > http://p.sf.net/sfu/intel-sw-dev > _______________________________________________ > Gcc4cli-devel mailing list > Gcc...@li... > https://lists.sourceforge.net/lists/listinfo/gcc4cli-devel |
From: Gabriele S. <gab...@gm...> - 2010-03-09 10:53:33
|
Hi Thierry, 2010/3/9 Thierry Lafage <thi...@in...>: > Hi Gabriele, > > Ok, sorry, I misunderstood the fixme "should lower only if vector types are > handled in this funtion." > I simply wanted, for easier debug, to ensure that the execution of this pass > could be controlled (ie. skipped by using a command-line argument). But it's > probably useless. such a flag might be useful but you should talk to Erven about it, he was the one who wrote the pass IIRC. > Ok, for this one, I've been very naive. Sorry. It seems to me that it has > been a fast copy/paste of a generic pass... I know, it certainly looked like it, in fact it was a rewrite of a pass we did on the tree representation before we introduced the CIL representation in the back-end. > For true/false, it happens that conditions on floating point values may be > computed by first using a comparison, and then a brtrue or brfalse. So > inverting the branch may lead to the same problems as with branches with FP > operands. OK, that must be why I've left it out. > In this particular case, we could look up at the operands of the > compare (if any) and invert the branch (or better: the compare itself?) only > if they are integer or pointers. Yes, it might work. Anyway the GCC testsuite will show you if there are any problems with this approach, I know it's very long and boring to run it but it's the best way to spot hard-to-find errors. I believe I tried switching the floating-point comparisons in the first place and switched back to integers only after checking with the testsuite myself. > For the FP general case, wouldn't it be possible to use *.un (or not) > versions of the branches? I'll make tests to check. Yes, if you are very careful you can change some of the conditions by using the appropriate unordered comparisons. However you should also verify with the mono JIT compiler if it is worth to do it: many architectures do not have the full range of floating-point ordered/unordered comparisons so on those architectures using them will result in very poor native code at JIT time. After considering all the problems I decided not to touch them because the chance of introducing some serious regressions was fairly high and the gain was very little. > However, concerning stack computation, since conditional branch inversion > does not change the stack at all, are the "cil_stack_after_stmt (stack, > stmt);" and "FOR_EACH_EDGE(...) { cil_set_stack_for_bb (...); }" useful? > I understand these statements as being a stack re-compute, isn't it? Yes, those compute the effect of an operation on the stack and propagate them to target basic-blocks at the end of the loop. We need them in order to know what are the types of the stack operands, however they are very cheap operations, I specifically designed them so that we don't need to fully recalculate the stack at every point. Instead cil_stack_after_stmt() will update the provided stack with only the changes caused by the specified instruction providing you with an up-to-date state of the stack. Gabriele |
From: Thierry L. <thi...@in...> - 2010-03-10 09:37:52
|
Hi Gabriele, > Hi Thierry, > >> For true/false, it happens that conditions on floating point values may be >> computed by first using a comparison, and then a brtrue or brfalse. So >> inverting the branch may lead to the same problems as with branches with FP >> operands. >> > > OK, that must be why I've left it out. > In fact, I've reverted my changes, and added cases to invert brtrue and brfalse because I still think it could work. Indeed, brtrue is the exact negation of brfalse whether a previous comparison occured on floats or not. I ran the testsuite, and it passed as well as before any change: 1247 unexpected fails, but all ieee tests that did not pass after my 1st patch now pass. So I think it's ok. >> For the FP general case, wouldn't it be possible to use *.un (or not) >> versions of the branches? I'll make tests to check. >> > > Yes, if you are very careful you can change some of the conditions by > using the appropriate unordered comparisons. However you should also > verify with the mono JIT compiler if it is worth to do it: many > architectures do not have the full range of floating-point > ordered/unordered comparisons so on those architectures using them > will result in very poor native code at JIT time. After considering > all the problems I decided not to touch them because the chance of > introducing some serious regressions was fairly high and the gain was > very little. > Despite it could result in poorer native code (when generating bytecode, we are not supposed to know the final target machine), inverting conditions in conditional branches still allows to reduce bytecode size (and, for some targets, it should speed up a little), so if I can do something for float comparisons, perhaps it would be worth adding another flag which would be activated with -Os, wouldn't it? I'll make some tests, then discuss with Erven (when he's cured), and finally come back to you with another, cleaner, patch. Regards, Thierry. |
From: Andrea C. O. <and...@gm...> - 2010-03-09 18:10:33
|
Hi, Sorry for the late replay. I have left only one remark (Gabriele and Erven already addressed all the others). We should probably avoid the changes in gcc/opts.c the right way of doing it is by defining OPTIMIZATION_OPTIONS in cil32.h as we did for OVERRIDE_OPTIONS You can look for an example on how to use them to gcc/config/cris/cris.[hc] it defines both OPTIMIZATION_OPTIONS and OVERRIDE_OPTIONS I do not remember exactly the difference of the two, so I do not know exactly where to put it, but the option does not exists for other targets. Andrea On Mon, Mar 8, 2010 at 5:39 PM, Thierry Lafage <thi...@in...> wrote: > Hi all, > > Here are several small patches for our gcc4cli BE branch: > > pr19340.c.patch is to avoid testing with pr19340.c because it has no sense > in cil32 since it tests the scheduler. > gcc4cli_line_info-t-cil32.patch fixes dependencies in t-cil32 and adds the > file source-location.h which was introduced in my last patch concerning > source location (this t-cil32 patch should have been part of the source > location patch). > cil-lower.c.patch modifies the pass gate in order to activate this pass only > when -ftree_vectorize option is activated (which is the case in -O3) > and simp-cond.patch is a simplification of the simp-cond pass which also > makes it more complete (adds support for brtrue/brfalse and for floating > point comparisons in conditional branches + add a command-line option which > is activated in -O1 + fixes missing brtrue/brfalse as being conditional > branch instructions). Note that this pass either does nothing, or change a > branch condition instruction into another one (inverts the condition) and > removes the following unconditional branch. This means that the stack is not > changed at all, so I removed all the stuff that was recomputing the stack at > every statement. Moreover, I didn't understand the test on the conditional > branch operand type (integer or pointer) in order to change the conditional > br instruction: I've removed it and tested on a real program, and it seems > to be ok. More tests are on the way (check-gcc + cbenchmarks). > > Feel free to comment. I should commit in a few days if you're ok (or if you > don't say anything). > > Regards, > > Thierry Lafage. > > Index: gcc/config/cil32/cil-lower.c > =================================================================== > --- gcc/config/cil32/cil-lower.c (revision 156016) > +++ gcc/config/cil32/cil-lower.c (working copy) > @@ -1,6 +1,6 @@ > /* CIL IR lowering. > > - Copyright (C) 2009 Free Software Foundation, Inc. > + Copyright (C) 2009-2010 Free Software Foundation, Inc. > > This file is part of GCC. > > @@ -23,11 +23,13 @@ > Andrea C. Ornstein > Erven Rohou > Gabriele Svelto > + Thierry Lafage > > Contact information at STMicroelectronics: > Andrea C. Ornstein <and...@st...> > Contact information at INRIA: > Erven Rohou <erv...@in...> > +Thierry Lafage <thi...@in...> > */ > > #include <stdio.h> > @@ -52,6 +54,8 @@ > * Globals > * > ******************************************************************************/ > > +extern int flag_tree_vectorize; // TL // This avoids including options.h > + > enum simd_backend simd_type = UNDEF_SIMD; > > /****************************************************************************** > @@ -591,8 +595,7 @@ > static bool > lower_cil_gate (void) > { > - /* FIXME: should lower only if vector types are handled in this funtion. > */ > - return true; > + return flag_tree_vectorize != 0; > } > > > > Index: gcc/config/cil32/t-cil32 > =================================================================== > --- gcc/config/cil32/t-cil32 (revision 156016) > +++ gcc/config/cil32/t-cil32 (working copy) > @@ -110,6 +110,7 @@ > $(srcdir)/config/cil32/cil-stmt.h \ > $(srcdir)/config/cil32/cil-types.h \ > $(srcdir)/config/cil32/emit-cil.h \ > + $(srcdir)/config/cil32/source-location.h \ > $(CONFIG_H) $(FLAGS_H) $(GGC_H) \ > $(SYSTEM_H) $(TIMEVAR_H) $(TM_H) $(TREE_H) $(TREE_FLOW_H) $(TREE_PASS_H) > \ > coretypes.h errors.h pointer-set.h > @@ -134,6 +135,7 @@ > missing-protos.o : $(srcdir)/config/cil32/missing-protos.c \ > $(srcdir)/config/cil32/cil-refs.h $(srcdir)/config/cil32/cil-stmt.h \ > $(srcdir)/config/cil32/cil-stmt-inline.h > $(srcdir)/config/cil32/cil-stack.h \ > + $(srcdir)/config/cil32/source-location.h \ > $(srcdir)/config/cil32/cil-types.h $(CONFIG_H) $(FLAGS_H) $(SYSTEM_H) \ > $(TIMEVAR_H) $(TM_H) $(TREE_H) $(TREE_PASS_H) coretypes.h errors.h \ > pointer-set.h > > Index: gcc/testsuite/gcc.dg/pr19340.c > =================================================================== > --- gcc/testsuite/gcc.dg/pr19340.c (revision 156016) > +++ gcc/testsuite/gcc.dg/pr19340.c (working copy) > @@ -1,6 +1,6 @@ > /* { dg-do compile } */ > /* { dg-options "-O1 -fschedule-insns2 -fsched2-use-traces" } */ > -/* { dg-skip-if "No scheduling" { mmix-*-* cris-*-* crisv32-*-* fido-*-* > m68k-*-* m32c-*-* avr-*-* } { "*" } { "" } } */ > +/* { dg-skip-if "No scheduling" { mmix-*-* cris-*-* crisv32-*-* fido-*-* > m68k-*-* m32c-*-* avr-*-* cil32-*-* } { "*" } { "" } } */ > > extern double f (double x); > > > Index: gcc/opts.c > =================================================================== > --- gcc/opts.c (revision 156016) > +++ gcc/opts.c (working copy) > @@ -914,6 +914,7 @@ > flag_tree_copy_prop = opt1; > flag_tree_sink = opt1; > flag_tree_ch = opt1; > + flag_cil32_simp_cond = opt1; > > /* -O2 optimizations. */ > opt2 = (optimize >= 2); > Index: gcc/config/cil32/cil32.opt > =================================================================== > --- gcc/config/cil32/cil32.opt (revision 156016) > +++ gcc/config/cil32/cil32.opt (working copy) > @@ -77,3 +77,7 @@ > msimd= > Target Joined RejectNegative Var(simd_backend_str) > -msimd=gcc|mono Select the SIMD backend > + > +fcil32simpcond > +Target Report Var(flag_cil32_simp_cond) Optimization ; added to -O1 > +Enable simplification of conditional expressions to make use of the fall > through path when possible > Index: gcc/config/cil32/simp-cond.c > =================================================================== > --- gcc/config/cil32/simp-cond.c (revision 156016) > +++ gcc/config/cil32/simp-cond.c (working copy) > @@ -2,7 +2,7 @@ > conditional/unconditional branches if one of the two outgoing paths falls > through to the next basic block. > > - Copyright (C) 2006-2009 Free Software Foundation, Inc. > + Copyright (C) 2006-2010 Free Software Foundation, Inc. > > This file is part of GCC. > > @@ -25,11 +25,13 @@ > Andrea Ornstein > Erven Rohou > Gabriele Svelto > + Thierry Lafage > > Contact information at STMicroelectronics: > Andrea C. Ornstein <and...@st...> > Contact information at INRIA: > Erven Rohou <erv...@in...> > +Thierry Lafage <thi...@in...> > */ > > #include "config.h" > @@ -45,11 +47,13 @@ > #include "cil-stmt.h" > #include "cil-types.h" > > +extern int flag_cil32_simp_cond; > + > /****************************************************************************** > * Local functions prototypes > * > ******************************************************************************/ > > -static void simplify_cond_branch (cil_stmt_iterator *, cil_stack); > +static void simplify_cond_branch (cil_stmt_iterator *); > static unsigned int simp_cond (void); > static bool simp_cond_gate (void); > > @@ -58,17 +62,18 @@ > ******************************************************************************/ > > static void > -simplify_cond_branch (cil_stmt_iterator *csi, cil_stack stack) > +simplify_cond_branch (cil_stmt_iterator *csi) > { > cil_stmt stmt; > - cil_type_t type; > enum cil_opcode opcode; > tree label_then, label_else; > - basic_block then_bb; > + basic_block cur_bb, then_bb; > edge true_edge, false_edge; > > + cur_bb = csi_bb (*csi); > + > /* Extract the condition's edges. */ > - extract_true_false_edges_from_block (csi_bb (*csi), &true_edge, > &false_edge); > + extract_true_false_edges_from_block (cur_bb, &true_edge, &false_edge); > label_then = gimple_block_label (true_edge->dest); > label_else = gimple_block_label (false_edge->dest); > > @@ -76,37 +81,34 @@ > > /* Simplify the condition only if the 'then' edge can be turned into a > fall through to the next basic block. */ > - if (csi_bb (*csi)->next_bb == then_bb) > + if (cur_bb->next_bb == then_bb) > { > - type = cil_stack_top (stack); > - > - if (cil_integer_p (type) || cil_pointer_p (type)) > + switch (cil_opcode (csi_stmt (*csi))) > { > - switch (cil_opcode (csi_stmt (*csi))) > - { > - case CIL_BEQ: opcode = CIL_BNE_UN; break; > - case CIL_BGE: opcode = CIL_BLT; break; > - case CIL_BGE_UN: opcode = CIL_BLT_UN; break; > - case CIL_BGT: opcode = CIL_BLE; break; > - case CIL_BGT_UN: opcode = CIL_BLE_UN; break; > - case CIL_BLE: opcode = CIL_BGT; break; > - case CIL_BLE_UN: opcode = CIL_BGT_UN; break; > - case CIL_BLT: opcode = CIL_BGE; break; > - case CIL_BLT_UN: opcode = CIL_BGE_UN; break; > - case CIL_BNE_UN: opcode = CIL_BEQ; break; > - default: > - return; /* Do nothing, we cannot change this branch. */ > - } > + case CIL_BEQ: opcode = CIL_BNE_UN; break; > + case CIL_BGE: opcode = CIL_BLT; break; > + case CIL_BGE_UN: opcode = CIL_BLT_UN; break; > + case CIL_BGT: opcode = CIL_BLE; break; > + case CIL_BGT_UN: opcode = CIL_BLE_UN; break; > + case CIL_BLE: opcode = CIL_BGT; break; > + case CIL_BLE_UN: opcode = CIL_BGT_UN; break; > + case CIL_BLT: opcode = CIL_BGE; break; > + case CIL_BLT_UN: opcode = CIL_BGE_UN; break; > + case CIL_BNE_UN: opcode = CIL_BEQ; break; > + case CIL_BRTRUE: opcode = CIL_BRFALSE; break; > + case CIL_BRFALSE: opcode = CIL_BRTRUE; break; > + default: > + return; /* Do nothing, we cannot change this branch. */ > + } > > - stmt = cil_build_stmt_arg (opcode, label_else); > - csi_replace (csi, stmt); > - csi_next (csi); > - csi_remove (csi); > + stmt = cil_build_stmt_arg (opcode, label_else); > + csi_replace (csi, stmt); > + csi_next (csi); > + csi_remove (csi); > > - /* Invert the out-going edges */ > - true_edge->flags ^= (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); > - false_edge->flags ^= (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); > - } > + /* Invert the out-going edges */ > + true_edge->flags ^= (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); > + false_edge->flags ^= (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); > } > } > > @@ -120,23 +122,14 @@ > basic_block bb; > cil_stmt_iterator csi; > cil_stmt stmt; > - enum cil_opcode opcode; > - cil_bb_stacks bbs; > - cil_stack stack; > - edge_iterator ei; > - edge e; > - > - bbs = cil_bb_stacks_create (); > > FOR_EACH_BB (bb) > { > - stack = cil_stack_for_bb (bbs, bb); > csi = csi_start_bb (bb); > > while (!csi_end_p (csi)) > { > stmt = csi_stmt (csi); > - opcode = cil_opcode (stmt); > > if (cil_cond_branch_p (stmt) && !csi_one_before_end_p (csi)) > { > @@ -145,22 +138,15 @@ > if (cil_opcode (csi_stmt (csi)) == CIL_BR) > { > csi_prev (&csi); > - simplify_cond_branch (&csi, stack); > + simplify_cond_branch (&csi); > break; > } > } > > - cil_stack_after_stmt (stack, stmt); > csi_next (&csi); > } > - > - FOR_EACH_EDGE (e, ei, bb->succs) > - { > - cil_set_stack_for_bb (bbs, e->dest, stack); > - } > } > > - cil_bb_stacks_destroy (bbs); > return 0; > } > > @@ -169,7 +155,7 @@ > static bool > simp_cond_gate (void) > { > - return true; > + return flag_cil32_simp_cond != 0; > } > > /* Define the parameters of the cond-simp pass. */ > Index: gcc/config/cil32/cil-stmt.c > =================================================================== > --- gcc/config/cil32/cil-stmt.c (revision 156016) > +++ gcc/config/cil32/cil-stmt.c (working copy) > @@ -1,6 +1,6 @@ > /* CIL statements, sequences and iterators implementation. > > - Copyright (C) 2006-2009 Free Software Foundation, Inc. > + Copyright (C) 2006-2010 Free Software Foundation, Inc. > > This file is part of GCC. > > @@ -23,11 +23,13 @@ > Andrea Ornstein > Erven Rohou > Gabriele Svelto > + Thierry Lafage > > Contact information at STMicroelectronics: > Andrea C. Ornstein <and...@st...> > Contact information at INRIA: > Erven Rohou <erv...@in...> > +Thierry Lafage <thi...@in...> > */ > > #include "config.h" > @@ -502,7 +504,9 @@ > case CIL_BLT: > case CIL_BLT_UN: > case CIL_BNE_UN: > - return true; > + case CIL_BRFALSE: > + case CIL_BRTRUE: > + return true; > > default: > return false; > > ------------------------------------------------------------------------------ > Download Intel® Parallel Studio Eval > Try the new software tools for yourself. Speed compiling, find bugs > proactively, and fine-tune applications for parallel performance. > See why Intel Parallel Studio got high marks during beta. > http://p.sf.net/sfu/intel-sw-dev > _______________________________________________ > Gcc4cli-devel mailing list > Gcc...@li... > https://lists.sourceforge.net/lists/listinfo/gcc4cli-devel > > |
From: Thierry L. <thi...@in...> - 2010-03-10 09:38:56
|
Hi Andrea, Thanks for your remark. I'll make use of OPTIMIZATION_OPTIONS: it's a macro which is called in opts.c (whereas OVERRIDE_OPTIONS is called from toplev.c). Thierry Lafage. Andrea Carlo Ornstein a écrit : > Hi, > Sorry for the late replay. > > I have left only one remark (Gabriele and Erven already addressed all > the others). > > We should probably avoid the changes in gcc/opts.c > the right way of doing it is by defining OPTIMIZATION_OPTIONS in > cil32.h as we did for OVERRIDE_OPTIONS > You can look for an example on how to use them to gcc/config/cris/cris.[hc] > it defines both OPTIMIZATION_OPTIONS and OVERRIDE_OPTIONS > > I do not remember exactly the difference of the two, so I do not know > exactly where to put it, but the option does not exists for other > targets. > > Andrea > > > > > On Mon, Mar 8, 2010 at 5:39 PM, Thierry Lafage <thi...@in...> wrote: > >> Hi all, >> >> Here are several small patches for our gcc4cli BE branch: >> >> pr19340.c.patch is to avoid testing with pr19340.c because it has no sense >> in cil32 since it tests the scheduler. >> gcc4cli_line_info-t-cil32.patch fixes dependencies in t-cil32 and adds the >> file source-location.h which was introduced in my last patch concerning >> source location (this t-cil32 patch should have been part of the source >> location patch). >> cil-lower.c.patch modifies the pass gate in order to activate this pass only >> when -ftree_vectorize option is activated (which is the case in -O3) >> and simp-cond.patch is a simplification of the simp-cond pass which also >> makes it more complete (adds support for brtrue/brfalse and for floating >> point comparisons in conditional branches + add a command-line option which >> is activated in -O1 + fixes missing brtrue/brfalse as being conditional >> branch instructions). Note that this pass either does nothing, or change a >> branch condition instruction into another one (inverts the condition) and >> removes the following unconditional branch. This means that the stack is not >> changed at all, so I removed all the stuff that was recomputing the stack at >> every statement. Moreover, I didn't understand the test on the conditional >> branch operand type (integer or pointer) in order to change the conditional >> br instruction: I've removed it and tested on a real program, and it seems >> to be ok. More tests are on the way (check-gcc + cbenchmarks). >> >> Feel free to comment. I should commit in a few days if you're ok (or if you >> don't say anything). >> >> Regards, >> >> Thierry Lafage. >> >> Index: gcc/config/cil32/cil-lower.c >> =================================================================== >> --- gcc/config/cil32/cil-lower.c (revision 156016) >> +++ gcc/config/cil32/cil-lower.c (working copy) >> @@ -1,6 +1,6 @@ >> /* CIL IR lowering. >> >> - Copyright (C) 2009 Free Software Foundation, Inc. >> + Copyright (C) 2009-2010 Free Software Foundation, Inc. >> >> This file is part of GCC. >> >> @@ -23,11 +23,13 @@ >> Andrea C. Ornstein >> Erven Rohou >> Gabriele Svelto >> + Thierry Lafage >> >> Contact information at STMicroelectronics: >> Andrea C. Ornstein <and...@st...> >> Contact information at INRIA: >> Erven Rohou <erv...@in...> >> +Thierry Lafage <thi...@in...> >> */ >> >> #include <stdio.h> >> @@ -52,6 +54,8 @@ >> * Globals >> * >> ******************************************************************************/ >> >> +extern int flag_tree_vectorize; // TL // This avoids including options.h >> + >> enum simd_backend simd_type = UNDEF_SIMD; >> >> /****************************************************************************** >> @@ -591,8 +595,7 @@ >> static bool >> lower_cil_gate (void) >> { >> - /* FIXME: should lower only if vector types are handled in this funtion. >> */ >> - return true; >> + return flag_tree_vectorize != 0; >> } >> >> >> >> Index: gcc/config/cil32/t-cil32 >> =================================================================== >> --- gcc/config/cil32/t-cil32 (revision 156016) >> +++ gcc/config/cil32/t-cil32 (working copy) >> @@ -110,6 +110,7 @@ >> $(srcdir)/config/cil32/cil-stmt.h \ >> $(srcdir)/config/cil32/cil-types.h \ >> $(srcdir)/config/cil32/emit-cil.h \ >> + $(srcdir)/config/cil32/source-location.h \ >> $(CONFIG_H) $(FLAGS_H) $(GGC_H) \ >> $(SYSTEM_H) $(TIMEVAR_H) $(TM_H) $(TREE_H) $(TREE_FLOW_H) $(TREE_PASS_H) >> \ >> coretypes.h errors.h pointer-set.h >> @@ -134,6 +135,7 @@ >> missing-protos.o : $(srcdir)/config/cil32/missing-protos.c \ >> $(srcdir)/config/cil32/cil-refs.h $(srcdir)/config/cil32/cil-stmt.h \ >> $(srcdir)/config/cil32/cil-stmt-inline.h >> $(srcdir)/config/cil32/cil-stack.h \ >> + $(srcdir)/config/cil32/source-location.h \ >> $(srcdir)/config/cil32/cil-types.h $(CONFIG_H) $(FLAGS_H) $(SYSTEM_H) \ >> $(TIMEVAR_H) $(TM_H) $(TREE_H) $(TREE_PASS_H) coretypes.h errors.h \ >> pointer-set.h >> >> Index: gcc/testsuite/gcc.dg/pr19340.c >> =================================================================== >> --- gcc/testsuite/gcc.dg/pr19340.c (revision 156016) >> +++ gcc/testsuite/gcc.dg/pr19340.c (working copy) >> @@ -1,6 +1,6 @@ >> /* { dg-do compile } */ >> /* { dg-options "-O1 -fschedule-insns2 -fsched2-use-traces" } */ >> -/* { dg-skip-if "No scheduling" { mmix-*-* cris-*-* crisv32-*-* fido-*-* >> m68k-*-* m32c-*-* avr-*-* } { "*" } { "" } } */ >> +/* { dg-skip-if "No scheduling" { mmix-*-* cris-*-* crisv32-*-* fido-*-* >> m68k-*-* m32c-*-* avr-*-* cil32-*-* } { "*" } { "" } } */ >> >> extern double f (double x); >> >> >> Index: gcc/opts.c >> =================================================================== >> --- gcc/opts.c (revision 156016) >> +++ gcc/opts.c (working copy) >> @@ -914,6 +914,7 @@ >> flag_tree_copy_prop = opt1; >> flag_tree_sink = opt1; >> flag_tree_ch = opt1; >> + flag_cil32_simp_cond = opt1; >> >> /* -O2 optimizations. */ >> opt2 = (optimize >= 2); >> Index: gcc/config/cil32/cil32.opt >> =================================================================== >> --- gcc/config/cil32/cil32.opt (revision 156016) >> +++ gcc/config/cil32/cil32.opt (working copy) >> @@ -77,3 +77,7 @@ >> msimd= >> Target Joined RejectNegative Var(simd_backend_str) >> -msimd=gcc|mono Select the SIMD backend >> + >> +fcil32simpcond >> +Target Report Var(flag_cil32_simp_cond) Optimization ; added to -O1 >> +Enable simplification of conditional expressions to make use of the fall >> through path when possible >> Index: gcc/config/cil32/simp-cond.c >> =================================================================== >> --- gcc/config/cil32/simp-cond.c (revision 156016) >> +++ gcc/config/cil32/simp-cond.c (working copy) >> @@ -2,7 +2,7 @@ >> conditional/unconditional branches if one of the two outgoing paths falls >> through to the next basic block. >> >> - Copyright (C) 2006-2009 Free Software Foundation, Inc. >> + Copyright (C) 2006-2010 Free Software Foundation, Inc. >> >> This file is part of GCC. >> >> @@ -25,11 +25,13 @@ >> Andrea Ornstein >> Erven Rohou >> Gabriele Svelto >> + Thierry Lafage >> >> Contact information at STMicroelectronics: >> Andrea C. Ornstein <and...@st...> >> Contact information at INRIA: >> Erven Rohou <erv...@in...> >> +Thierry Lafage <thi...@in...> >> */ >> >> #include "config.h" >> @@ -45,11 +47,13 @@ >> #include "cil-stmt.h" >> #include "cil-types.h" >> >> +extern int flag_cil32_simp_cond; >> + >> /****************************************************************************** >> * Local functions prototypes >> * >> ******************************************************************************/ >> >> -static void simplify_cond_branch (cil_stmt_iterator *, cil_stack); >> +static void simplify_cond_branch (cil_stmt_iterator *); >> static unsigned int simp_cond (void); >> static bool simp_cond_gate (void); >> >> @@ -58,17 +62,18 @@ >> ******************************************************************************/ >> >> static void >> -simplify_cond_branch (cil_stmt_iterator *csi, cil_stack stack) >> +simplify_cond_branch (cil_stmt_iterator *csi) >> { >> cil_stmt stmt; >> - cil_type_t type; >> enum cil_opcode opcode; >> tree label_then, label_else; >> - basic_block then_bb; >> + basic_block cur_bb, then_bb; >> edge true_edge, false_edge; >> >> + cur_bb = csi_bb (*csi); >> + >> /* Extract the condition's edges. */ >> - extract_true_false_edges_from_block (csi_bb (*csi), &true_edge, >> &false_edge); >> + extract_true_false_edges_from_block (cur_bb, &true_edge, &false_edge); >> label_then = gimple_block_label (true_edge->dest); >> label_else = gimple_block_label (false_edge->dest); >> >> @@ -76,37 +81,34 @@ >> >> /* Simplify the condition only if the 'then' edge can be turned into a >> fall through to the next basic block. */ >> - if (csi_bb (*csi)->next_bb == then_bb) >> + if (cur_bb->next_bb == then_bb) >> { >> - type = cil_stack_top (stack); >> - >> - if (cil_integer_p (type) || cil_pointer_p (type)) >> + switch (cil_opcode (csi_stmt (*csi))) >> { >> - switch (cil_opcode (csi_stmt (*csi))) >> - { >> - case CIL_BEQ: opcode = CIL_BNE_UN; break; >> - case CIL_BGE: opcode = CIL_BLT; break; >> - case CIL_BGE_UN: opcode = CIL_BLT_UN; break; >> - case CIL_BGT: opcode = CIL_BLE; break; >> - case CIL_BGT_UN: opcode = CIL_BLE_UN; break; >> - case CIL_BLE: opcode = CIL_BGT; break; >> - case CIL_BLE_UN: opcode = CIL_BGT_UN; break; >> - case CIL_BLT: opcode = CIL_BGE; break; >> - case CIL_BLT_UN: opcode = CIL_BGE_UN; break; >> - case CIL_BNE_UN: opcode = CIL_BEQ; break; >> - default: >> - return; /* Do nothing, we cannot change this branch. */ >> - } >> + case CIL_BEQ: opcode = CIL_BNE_UN; break; >> + case CIL_BGE: opcode = CIL_BLT; break; >> + case CIL_BGE_UN: opcode = CIL_BLT_UN; break; >> + case CIL_BGT: opcode = CIL_BLE; break; >> + case CIL_BGT_UN: opcode = CIL_BLE_UN; break; >> + case CIL_BLE: opcode = CIL_BGT; break; >> + case CIL_BLE_UN: opcode = CIL_BGT_UN; break; >> + case CIL_BLT: opcode = CIL_BGE; break; >> + case CIL_BLT_UN: opcode = CIL_BGE_UN; break; >> + case CIL_BNE_UN: opcode = CIL_BEQ; break; >> + case CIL_BRTRUE: opcode = CIL_BRFALSE; break; >> + case CIL_BRFALSE: opcode = CIL_BRTRUE; break; >> + default: >> + return; /* Do nothing, we cannot change this branch. */ >> + } >> >> - stmt = cil_build_stmt_arg (opcode, label_else); >> - csi_replace (csi, stmt); >> - csi_next (csi); >> - csi_remove (csi); >> + stmt = cil_build_stmt_arg (opcode, label_else); >> + csi_replace (csi, stmt); >> + csi_next (csi); >> + csi_remove (csi); >> >> - /* Invert the out-going edges */ >> - true_edge->flags ^= (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); >> - false_edge->flags ^= (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); >> - } >> + /* Invert the out-going edges */ >> + true_edge->flags ^= (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); >> + false_edge->flags ^= (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); >> } >> } >> >> @@ -120,23 +122,14 @@ >> basic_block bb; >> cil_stmt_iterator csi; >> cil_stmt stmt; >> - enum cil_opcode opcode; >> - cil_bb_stacks bbs; >> - cil_stack stack; >> - edge_iterator ei; >> - edge e; >> - >> - bbs = cil_bb_stacks_create (); >> >> FOR_EACH_BB (bb) >> { >> - stack = cil_stack_for_bb (bbs, bb); >> csi = csi_start_bb (bb); >> >> while (!csi_end_p (csi)) >> { >> stmt = csi_stmt (csi); >> - opcode = cil_opcode (stmt); >> >> if (cil_cond_branch_p (stmt) && !csi_one_before_end_p (csi)) >> { >> @@ -145,22 +138,15 @@ >> if (cil_opcode (csi_stmt (csi)) == CIL_BR) >> { >> csi_prev (&csi); >> - simplify_cond_branch (&csi, stack); >> + simplify_cond_branch (&csi); >> break; >> } >> } >> >> - cil_stack_after_stmt (stack, stmt); >> csi_next (&csi); >> } >> - >> - FOR_EACH_EDGE (e, ei, bb->succs) >> - { >> - cil_set_stack_for_bb (bbs, e->dest, stack); >> - } >> } >> >> - cil_bb_stacks_destroy (bbs); >> return 0; >> } >> >> @@ -169,7 +155,7 @@ >> static bool >> simp_cond_gate (void) >> { >> - return true; >> + return flag_cil32_simp_cond != 0; >> } >> >> /* Define the parameters of the cond-simp pass. */ >> Index: gcc/config/cil32/cil-stmt.c >> =================================================================== >> --- gcc/config/cil32/cil-stmt.c (revision 156016) >> +++ gcc/config/cil32/cil-stmt.c (working copy) >> @@ -1,6 +1,6 @@ >> /* CIL statements, sequences and iterators implementation. >> >> - Copyright (C) 2006-2009 Free Software Foundation, Inc. >> + Copyright (C) 2006-2010 Free Software Foundation, Inc. >> >> This file is part of GCC. >> >> @@ -23,11 +23,13 @@ >> Andrea Ornstein >> Erven Rohou >> Gabriele Svelto >> + Thierry Lafage >> >> Contact information at STMicroelectronics: >> Andrea C. Ornstein <and...@st...> >> Contact information at INRIA: >> Erven Rohou <erv...@in...> >> +Thierry Lafage <thi...@in...> >> */ >> >> #include "config.h" >> @@ -502,7 +504,9 @@ >> case CIL_BLT: >> case CIL_BLT_UN: >> case CIL_BNE_UN: >> - return true; >> + case CIL_BRFALSE: >> + case CIL_BRTRUE: >> + return true; >> >> default: >> return false; >> >> ------------------------------------------------------------------------------ >> Download Intel® Parallel Studio Eval >> Try the new software tools for yourself. Speed compiling, find bugs >> proactively, and fine-tune applications for parallel performance. >> See why Intel Parallel Studio got high marks during beta. >> http://p.sf.net/sfu/intel-sw-dev >> _______________________________________________ >> Gcc4cli-devel mailing list >> Gcc...@li... >> https://lists.sourceforge.net/lists/listinfo/gcc4cli-devel >> >> >> |
From: Thierry L. <thi...@in...> - 2010-03-12 16:20:57
|
Hi all, This is just to says that patches pr19340.c.patch and gcc4cli_line_info-t-cil32.patch have been commited in our branch. Thierry Lafage. Thierry Lafage a écrit : > Hi all, > > Here are several small patches for our gcc4cli BE branch: > > * *pr19340.c.patch* is to avoid testing with pr19340.c because it > has no sense in cil32 since it tests the scheduler. > * *gcc4cli_line_info-t-cil32.patch* fixes dependencies in t-cil32 > and adds the file source-location.h which was introduced in my > last patch concerning source location (this t-cil32 patch should > have been part of the source location patch). > * *cil-lower.c.patch* modifies the pass gate in order to activate > this pass only when -ftree_vectorize option is activated (which > is the case in -O3) > * and *simp-cond.patch* is a simplification of the simp-cond pass > which also makes it more complete (adds support for > brtrue/brfalse and for floating point comparisons in conditional > branches + add a command-line option which is activated in -O1 + > fixes missing brtrue/brfalse as being conditional branch > instructions). Note that this pass either does nothing, or > change a branch condition instruction into another one (inverts > the condition) and removes the following unconditional branch. > This means that the stack is not changed at all, so I removed > all the stuff that was recomputing the stack at every statement. > Moreover, I didn't understand the test on the conditional branch > operand type (integer or pointer) in order to change the > conditional br instruction: I've removed it and tested on a real > program, and it seems to be ok. More tests are on the way > (check-gcc + cbenchmarks). > > Feel free to comment. I should commit in a few days if you're ok (or > if you don't say anything). > > Regards, > Thierry Lafage. > > ------------------------------------------------------------------------ > > ------------------------------------------------------------------------------ > Download Intel® Parallel Studio Eval > Try the new software tools for yourself. Speed compiling, find bugs > proactively, and fine-tune applications for parallel performance. > See why Intel Parallel Studio got high marks during beta. > http://p.sf.net/sfu/intel-sw-dev > ------------------------------------------------------------------------ > > _______________________________________________ > Gcc4cli-devel mailing list > Gcc...@li... > https://lists.sourceforge.net/lists/listinfo/gcc4cli-devel |