Hi Richard, I addressed all your comments but the ones below.
> From: Richard Biener [mailto:richard.guent...@gmail.com] > > + /* Convert the result of load if necessary. */ > + if (!useless_type_conversion_p (TREE_TYPE (tgt), > + TREE_TYPE (val_tmp))) > + { > > why's that? Shouldn't the load already be emitted of the correct type? The size would be correct and I wasn't sure if the sign matters. I've read useless_type_conversion_p since and it seems I was overly worried. > > You seem to replace the stmt computing the target value by directly > loading into the target. IMHO that's premature optimization and it > would be easier to just replace its rhs (that way the stmt still has > a proper location for example). Right. I merely followed what was already done and indeed the whole statement is replaced. I don't understand why you say it's a premature optimization. Since the left hand side is kept, isn't it equivalent (lost of location excluded)? I'll change to avoid such a replacement. > > And now that I am looking and the patch series again - I think > you need to verify that you load the correct value. Consider > > int foo (char *x, char *y) > { > char c0 = x[0]; > char c1 = x[1]; > *y = 1; > char c2 = x[2]; > char c3 = x[3]; > return c0 | c1 << 8 | c2 << 16 | c3 << 24; > } > > where do you insert the single load? The easiest way out > (without doing alias walks and whatnot) is to verify that all > loads have the same gimple_vuse () attached (also please > set that gimple_vuse () on the load you build - that avoids > costly computation of virtual operands). Nice catch. I'll do that. Thanks for you answer and don't worry for the delay: I have other things to keep me busy and stage1 is not closed yet. Best regards, Thomas
