[PATCH][4.1/4.2] Fix PR27260, fallout of fix for PR27095
Tue Apr 25 23:46:00 GMT 2006
On Sat, Apr 22, 2006 at 05:04:21PM +0200, Richard Guenther wrote:
> This fixes PR27260 by making sure to expand the value for memset in
> the original mode rather than QI mode.
I'll add some commentary here while the issues are fresh in my mind.
Also a question. Do we really need to bother with builtin_save_expr for
The basic problem we need to solve in expand_builtin_memset is how to
safely expand trees to rtl twice, or to avoid such expansion.
expand_builtin_memset's inputs are trees and a function called therein,
set_storage_via_setmem, works with rtl. set_storage_via_setmem might
not always succeed, and there is no predicate like can_store_by_pieces
that can tell us before expansion that it will succeed. On failure of
the various optimised means of implementing builtin_memset, we fall back
to a memset libcall which also is expanded from trees. So it seems we
need to ensure that we can safely expand the arg trees twice without
running into pr27095, or we need to write a can_set_storage_via_setmem.
I don't fancy writing the latter.
Expanding arg trees twice isn't simply a matter of discarding the rtl
from the first expansion, then trying again. If the tree contains a
SAVE_EXPR, then the sub-tree under the SAVE_EXPR won't be expanded the
second time. Instead, the second expansion will reuse the rtl generated
in the first expansion. This means we can't throw rtl away willy-nilly,
or if we do, the saves need to be undone. See 3.4 unsafe_for_reeval
for even more dirty details. 4.0 and up are much simplified due to ssa.
After much looking at code, I don't think we'll ever see a SAVE_EXPR in
the arg trees. expand_call seems to agree with my analysis, with
fix_unsafe_tree no longer needed.
Now, the reason why PR27260 occurs is that an outer CONVERT_EXPR causes
the inner sub-tree to be expanded in a smaller mode, with the SAVE_EXPR
keeping the rtl and reusing in the second expansion. Hence the mode is
wrong for the second expansion. That seems to be "just the way
SAVE_EXPR works", and changing it would likely lose some optimisations.
IBM OzLabs - Linux Technology Centre
More information about the Gcc-patches