On Fri, Jun 24, 2011 at 3:46 PM, Stubbs, Andrew <andrew_stu...@mentor.com> wrote: > On 24/06/11 09:28, Richard Guenther wrote: >>> > To be clear, it only skips past NOP_EXPR. Is it not the case that what >>> > you're describing would need a CONVERT_EXPR? >> NOP_EXPR is the same as CONVERT_EXPR. > > Are you sure?
Yes, definitely. They are synonyms of each other (an unfinished merging process), the usual check for them is via CONVERT_EXPR_P. > I thought this was safe because the internals manual says: > > NOP_EXPR > These nodes are used to represent conversions that do not require any > code-generation .... > > CONVERT_EXPR > These nodes are similar to NOP_EXPRs, but are used in those > situations where code may need to be generated .... Which is wrong (sorry). > So, I tried this example: > > int > foo (int a, short b, short c) > { > int bc = b * c; > return a + (short)bc; > } > > Both before and after my patch, GCC gives: > > mul r2, r1, r2 > sxtah r0, r0, r2 > > (where, SXTAH means sign-extend the third operand from HImode to SImode > and add to the second operand.) > > The dump after the widening_mult pass is: > > foo (int a, short int b, short int c) > { > int bc; > int D.2018; > short int D.2017; > int D.2016; > int D.2015; > int D.2014; > > <bb 2>: > D.2014_2 = (int) b_1(D); > D.2015_4 = (int) c_3(D); > bc_5 = b_1(D) w* c_3(D); > D.2017_6 = (short int) bc_5; > D.2018_7 = (int) D.2017_6; > D.2016_9 = D.2018_7 + a_8(D); > return D.2016_9; > > } > > Where you can clearly see that the addition has not been recognised as a > multiply-and-accumulate. > > When I step through convert_plusminus_to_widen, I can see that the > reason it has not matched is because "D.2017_6 = (short int) bc_5" is > encoded with a CONVERT_EXPR, just as the manual said it would be. A NOP_EXPR in this place would be valid as well. The merging hasn't been completed and at least the C frontend still generates CONVERT_EXPRs in some cases. > So, according to the manual, and my (admittedly limited) experiments, > skipping over NOP_EXPR does appear to be safe. > > But you say that it isn't safe. So now I'm confused. :( > > I can certainly add checks to make sure that the skipped operations > actually don't make any important changes to the value, but do I need to? Yes. Thanks, Richard. > Andrew >