----- Original Message ----- > On Mon, 8 Jul 2013, Kai Tietz wrote: > > > These passes are implementing type-demotion (type sinking into > > statements) for some gimple statements. I limitted inital > > implementation to the set of multiply, addition, substraction, and > > binary-and/or/xor. Additional this pass adds some rules to sink > > type-cast sequences - eg. (int) (short) x; with char as type of x. > > This special handing in this pass of such type-sequence simplification > > is necessary to avoid too complex cast-sequences by type-unsigned > > conversion used by this pass to avoid undefined overflow behaviour. > > Hello, > > thanks for working on this. This seems like a nice chance for me to learn, > so I am trying to understand your patch, and I would be glad if you could > give me a couple hints.
I try. > I wonder why you implemented this as a separate pass instead of adding it > to tree-ssa-forwprop. demote_cast is (or should be) a special case of > combine_conversions, so it would be nice to avoid the code duplication > (there is also a version in fold-const.c). demote_into could be called > from roughly the same place as simplify_conversion_from_bitmask. And you > could reuse get_prop_source_stmt, can_propagate_from, > remove_prop_source_from_use, etc. Initial patch (from last year) actual implemented that in forwprop. I was then kindly asked to put this into a separate pass. There are some good reason why forward-propagation isn't the right place for it. Eg, forwprop does type-raising by default. So by implementing type-demotion there too, would lead to raise-condition. So there would be required additionally that within forwprop a straight line-depth conversion is done for statement-lists. All this doesn't fit pretty well into current concept of forward-propagation ... The cast demotion is of course something of interest for folding and might be fitting into forward-propagation-pass too. The main cause why it is implemented within demotion pass is, that this pass introduces such cast-demotion-folding opportunities due its "unsigned"-type expansion. So we want to fold that within pass and not waiting until a later pass optimizes such redundant sequences away. Btw, the cast-demotion can be still improved for some cases - see here as example the gcc.target/i386/rotate-1.c testcase for such an improvement - so that some expressions of the form of (type1) (((type2) x) op ((type2) y)) can be transformed into x op y. > If I understand, the main reason is because you want to go through the > statements in reverse order, since this is the way the casts are being > propagated (would forwprop also work, just more slowly, or would it miss > opportunities across basic blocks?). It would miss some opportunities, and causes penalty on speed due concept of forwprop. > I have some trouble understanding why something as complicated as > build_and_add_sum (which isn't restricted to additions by the way) is > needed. Could you add a comment to the code explaining why you need to > insert the statements precisely there and not for instance just before > their use? Is that to try and help CSE? Yes, this function is a bit more complex as actual required for now in general. Nevertheless required. And I expect that demotion-pass will improve and so even the right now "unlikely" cases might be required more frequent. I had in front lighter version of statement addition, but it failed in some cases.In some (rare) cases we would otherwise choose wrong basic-block to add the new "cast"-statements. Well, there is another place (reassoc) where you can find nearly the same function, and its needs are exactly the same as within demote-pass. > > I have added an additional early pass "typedemote1" to this patch for > > simple cases types can be easily sunken into statement without special > > unsigned-cast for overflow-case. Jakub asked for it. Tests have shown > > that this pass does optimizations in pretty few cases. As example in > > testsuite see for example pr46867.c testcase. > > The second pass (I put it behind first vrp pass to avoid > > testcase-conflicts) uses 'unsigned'-type casts to avoid undefined overflow > > behavior. This pass has much more hits in standard code, > > I assume that, when the pass is done, we can consider removing the > corresponding code from the front-ends? That would increase the hits ;-) Hmm, well possible, but unlikely. FE and ME are two pretty different things. Sadly stuff isn't as clearly separated as it should be. But well, I know there is work going on to achieve that in this area. > -- > Marc Glisse > Kai