Message ID | 517EE43B.90909@codesourcery.com |
---|---|
State | New |
Headers | show |
On Mon, Apr 29, 2013 at 11:20 PM, Bernd Schmidt <bernds@codesourcery.com> wrote: > Currently, MEM_REF contains two pointer arguments, one which is supposed > to be a base object and another which is supposed to be a constant > offset. This representation is somewhat problematic, as not all machines > treat pointer values as essentially integers. On machines where size_t > is smaller than a pointer, for example m32c where it's due to > limitations in the compiler, or the port I've been working on recently > where pointers contain a segment selector that does not participate in > additions, this is not an accurate representation, and it does cause > real issues. > > It would be better to use a representation more like POINTER_PLUS with a > pointer and a real sizetype integer. Can someone explain the comment in > tree.def which states that the type of the constant offset is used for > TBAA purposes? It states "MEM_REF <p, c> is equivalent to > ((typeof(c))p)->x [...]", so why not represent it as MEM_REF <(desired > type)p, (size_t)c>? Because if p is not of desired type then we have to emit a separate stmt with a type conversion (which are all useless now, btw). Initially I played with having an extra type operand, but all hell breaks lose if you have tree->exp.operand[] be a TREE_TYPE. So I settled for the convenient place of using the constants type. > The following patch works around one instance of the problem. When we > fold an offset addition, the addition must be performed in sizetype, > otherwise we may get unwanted overflow. This bug triggers on m32c for > example, where an offset of 65528 (representing -8) and and offset of 8 > are added, yielding an offset of 65536 instead of zero. Solved by > performing the intermediate computation in sizetype. Ah, yes. Note that this is why we have mem_ref_offset () in tree.c. So a better fix would be to use return fold_build2 (MEM_REF, type, build_fold_addr_expr (base), double_int_to_tree (type1, tree_to_double_int (arg1).sext (TYPE_PRECISION (TREE_TYPE (arg1))) + coffset)); that is, perform the arithmetic using double_int. Note that + arg1 = fold_convert (size_type_node, arg1); would no longer sign-extend arg1, you'd need to use ssize_type_node. And using [s]size_type_node for the offset would wreck targets that have 16bit sizetype and 24bit pointer type (it would truncate parts of the offset). Richard. > Bootstrapped and tested on x86_64-linux (all languages except Ada) with > no changes in the tests, and tested on m32c-elf where it fixes 22 > failures. Ok? > > Bernd
* fold-const.c (fold_binary_loc): When folding an addition in the offset of a memref, use size_type to perform the arithmetic. diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 59dbc03..6f092ab 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -10025,15 +10025,17 @@ fold_binary_loc (location_t loc, && handled_component_p (TREE_OPERAND (arg0, 0))) { tree base; + tree type1 = TREE_TYPE (arg1); HOST_WIDE_INT coffset; base = get_addr_base_and_unit_offset (TREE_OPERAND (arg0, 0), &coffset); if (!base) return NULL_TREE; - return fold_build2 (MEM_REF, type, - build_fold_addr_expr (base), - int_const_binop (PLUS_EXPR, arg1, - size_int (coffset))); + arg1 = fold_convert (size_type_node, arg1); + arg1 = int_const_binop (PLUS_EXPR, arg1, size_int (coffset)); + base = build_fold_addr_expr (base); + arg1 = fold_convert (type1, arg1); + return fold_build2 (MEM_REF, type, base, arg1); } return NULL_TREE;