Richard Biener wrote:
> On Fri, Mar 8, 2013 at 3:19 PM, Richard Biener
> <richard.guent...@gmail.com> wrote:
>> On Fri, Mar 8, 2013 at 2:57 PM, Georg-Johann Lay wrote:
>>> While implementing PR56263 (Strict address-space checking for AVR), I
>>> encountered the problem that pointer casts with address spaces are always
>>> expanded as const_int 0.
>>>
>>> The problem occurs if the attached patch that implements PR56263 and the
>>> following code is compiled as
>>>
>>> $ avr-gcc -Os flash-cast.c -S -mstrict-addr-space-subsets
>>>
>>>
>>> #define PROGMEM __attribute__((__progmem__))
>>>
>>> #define PSTR(s) \
>>> (__extension__ \
>>> ({ \
>>> static const char __c[] PROGMEM = (s); \
>>> &__c[0]; \
>>> }))
>>>
>>> extern void print (const __memx char*, ...);
>>>
>>> const __flash char *p;
>>>
>>> void f (const char *c)
>>> {
>>> c = (const char*) p;
>>>
>>> print ((const __flash char*) PSTR ("Hallo flash"));
>>> }
>>>
>>>
>>>
>>> The corresponding .expand dump reads:
>>>
>>>
>>> f (const char * c)
>>> {
>>> static const char __c[12] = "Hallo flash";
>>> const <address-space-1> char * _2;
>>> const <address-space-7> char * _3;
>>>
>>> ;; basic block 2, loop depth 0
>>> ;; pred: ENTRY
>>> _2 = (const <address-space-1> char *) (&__c[0]);
>>> _3 = (const <address-space-7> char *) _2;
>>> print (_3); [tail call]
>>> return;
>>> ;; succ: EXIT
>>>
>>> }
>>>
>>>
>>> but the expression is always emit as (const_int 0).
>>>
>>>
>>> Trying to track this issue, I ended up in build1_stat which enters the
>>> default
>>> case for "code" (ADDR_SPACE_CONVERT_EXPR at that time) and returns the
>>> following tree:
>>>
>>>
>>> (gdb) p t
>>> $62 = (tree) 0xb7bb4578
>>> (gdb) pt
>>> <addr_space_convert_expr 0xb7bb4578
>>> type <pointer_type 0xb7bc3a80
>>> type <integer_type 0xb7bc3a20 char readonly sizes-gimplified
>>> address-space-1 string-flag QI
>>> size <integer_cst 0xb7b3a1a4 constant 8>
>>> unit size <integer_cst 0xb7b3a1b8 constant 1>
>>> align 8 symtab 0 alias set -1 canonical type 0xb7bc3a20
>>> precision 8
>>> min <integer_cst 0xb7b3a1e0 -128> max <integer_cst 0xb7b3a208 127>
>>> pointer_to_this <pointer_type 0xb7bc3a80>>
>>> unsigned HI
>>> size <integer_cst 0xb7b3a08c constant 16>
>>> unit size <integer_cst 0xb7b3a0a0 constant 2>
>>> align 8 symtab 0 alias set -1 canonical type 0xb7bc3a80>
>>> readonly constant
>>> arg 0 <addr_expr 0xb7bb449c
>>> type <pointer_type 0xb7b4f2a0 type <integer_type 0xb7b4f240 char>
>>> public unsigned HI size <integer_cst 0xb7b3a08c 16> unit size
>>> <integer_cst 0xb7b3a0a0 2>
>>> align 8 symtab 0 alias set -1 canonical type 0xb7b4f2a0
>>> pointer_to_this <pointer_type 0xb7b4f840>>
>>> readonly constant
>>> arg 0 <array_ref 0xb7b465a0 type <integer_type 0xb7b4f240 char>
>>> readonly arg 0 <var_decl 0xb7bcd05c __c>
>>> arg 1 <integer_cst 0xb7b3a460 constant 0>
>>> flash-cast.c:18:128>
>>> flash-cast.c:18:124>>
>>>
>>> Problem is the arg 1 <integer_cst 0xb7b3a460 constant 0> at the end which
>>> leads
>>> to the expansion of 0.
>>>
>>> The call chain is:
>>>
>>> #0 build1_stat (code=ADDR_SPACE_CONVERT_EXPR, type=0xb7bc3a80,
>>> node=0xb7bb449c) at ../../../gcc.gnu.org/trunk/gcc/tree.c:3848
>>> (gdb) bt
>>> #0 build1_stat (code=ADDR_SPACE_CONVERT_EXPR, type=0xb7bc3a80,
>>> node=0xb7bb449c) at ../../../gcc.gnu.org/trunk/gcc/tree.c:3848
>>> #1 0x08286be0 in gimple_assign_rhs_to_tree (stmt=0xb7bc2c30) at
>>> ../../../gcc.gnu.org/trunk/gcc/cfgexpand.c:86
>>> #2 0x0837047f in expand_expr_real_1 (exp=0xb7b972f8, target=0x0,
>>> tmode=VOIDmode, modifier=EXPAND_STACK_PARM, alt_rtl=0x0) at
>>> ../../../gcc.gnu.org/trunk/gcc/expr.c:9274
>>> #3 0x083762f2 in expand_expr_real (exp=0xb7b972f8, target=0x0,
>>> tmode=VOIDmode,
>>> modifier=EXPAND_STACK_PARM, alt_rtl=0x0) at
>>> ../../../gcc.gnu.org/trunk/gcc/expr.c:7863
>>> #4 0x08376a7e in expand_expr (exp=0xb7b972f8, target=0x0, mode=VOIDmode,
>>> modifier=EXPAND_STACK_PARM) at ../../../gcc.gnu.org/trunk/gcc/expr.h:444
>>> #5 0x0836ae96 in expand_expr_real_2 (ops=0xbfffca10, target=0x0,
>>> tmode=VOIDmode, modifier=EXPAND_STACK_PARM) at
>>> ../../../gcc.gnu.org/trunk/gcc/expr.c:8150
>>> #6 0x08376234 in expand_expr_real_1 (exp=0xb7bb4564, target=0x0,
>>> tmode=VOIDmode, modifier=EXPAND_STACK_PARM, alt_rtl=0x0) at
>>> ../../../gcc.gnu.org/trunk/gcc/expr.c:10491
>>> #7 0x083762f2 in expand_expr_real (exp=0xb7bb4564, target=0x0,
>>> tmode=VOIDmode,
>>> modifier=EXPAND_STACK_PARM, alt_rtl=0x0) at
>>> ../../../gcc.gnu.org/trunk/gcc/expr.c:7863
>>> #8 0x083704a6 in expand_expr_real_1 (exp=0xb7b97320, target=0x0,
>>> tmode=VOIDmode, modifier=EXPAND_STACK_PARM, alt_rtl=0x0) at
>>> ../../../gcc.gnu.org/trunk/gcc/expr.c:9274
>>> #9 0x083762f2 in expand_expr_real (exp=0xb7b97320, target=0x0,
>>> tmode=VOIDmode,
>>> modifier=EXPAND_STACK_PARM, alt_rtl=0x0) at
>>> ../../../gcc.gnu.org/trunk/gcc/expr.c:7863
>>> #10 0x0826ae1a in expand_expr (exp=0xb7b97320, target=0x0, mode=VOIDmode,
>>> modifier=EXPAND_STACK_PARM) at ../../../gcc.gnu.org/trunk/gcc/expr.h:444
>>> #11 0x0826c215 in store_one_arg (arg=0xbfffd130, argblock=0x0, flags=0,
>>> variable_size=0, reg_parm_stack_space=0) at
>>> ../../../gcc.gnu.org/trunk/gcc/calls.c:4500
>>> #12 0x08274d79 in expand_call (exp=0xb7b46640, target=0x0, ignore=1) at
>>> ../../../gcc.gnu.org/trunk/gcc/calls.c:3040
>>> #13 0x08375088 in expand_expr_real_1 (exp=0xb7b46640, target=0x0,
>>> tmode=VOIDmode, modifier=EXPAND_NORMAL, alt_rtl=0x0) at
>>> ../../../gcc.gnu.org/trunk/gcc/expr.c:10209
>>> #14 0x0828cb86 in expand_call_stmt (stmt=0xb7fde140) at
>>> ../../../gcc.gnu.org/trunk/gcc/cfgexpand.c:2114
>>>
>>> ...
>>>
>>>
>>> The problem comes apparent in #5 expand_expr_real_2
>>>
>>> if (targetm.addr_space.subset_p (as_to, as_from)
>>> || targetm.addr_space.subset_p (as_from, as_to))
>>> {
>>> op0 = expand_expr (treeop0, NULL_RTX, VOIDmode, modifier);
>>> op0 = targetm.addr_space.convert (op0, treeop0_type, type);
>>> gcc_assert (op0);
>>> return op0;
>>> }
>>>
>>> where op0 is const0_rtx and targetm.addr_space.convert cannot recover from
>>> that.
>>>
>>> Maybe someone can help me fixing the root cause?
>> Doesn't make sense - in the above code treeop0 should be the
>>
>> arg 0 <addr_expr 0xb7bb449c
>>
>> not the zero integer constant.
>
> The first conversion expands to
>
> (symbol_ref:HI ("__c.1466") [flags 0x202] <var_decl 0x7ffff6dfaa18 __c>)
>
> then we try to convert that to another address-space and the target hook
> returns
>
> (insn 6 0 7 (set (reg/f:HI 46)
> (symbol_ref:HI ("__c.1466") [flags 0x202] <var_decl
> 0x7ffff6dfaa18 __c>)) t.c:18 -1
> (nil))
>
> (insn 7 6 0 (set (reg:PSI 45)
> (zero_extend:PSI (reg/f:HI 46))) t.c:18 -1
> (nil))
>
> (reg:PSI 45)
>
> so the issue must be elsewhere (in case the above is correct).
This looks correct. Without the patch you'll have always
-mno-strict-addr-space-subsets where the code is good.
> -mstrict-addr-space-subsets
>
> doesn't exist for me.
You must apply the patch "strict-addr-spaces.diff" from the initial mail. This
adds the -mstrict-addr-space-subsets option so the user can pick strictness of
address space checking (PR56263).
For a quick change you can change avr.c from
static bool
avr_addr_space_subset_p (addr_space_t subset ATTRIBUTE_UNUSED,
addr_space_t superset ATTRIBUTE_UNUSED)
{
return true;
}
to
static bool
avr_addr_space_subset_p (addr_space_t subset ATTRIBUTE_UNUSED,
addr_space_t superset)
{
return superset == ADDR_SPACE_MEMX;
}
The C code contains 2 casts: The first casts a pointer to generic space in
PSTR ("Hallo world") to a 16-bit __flash pointer. The second cast is to a
24-bit __memx pointer which is for the print() prototype.
PSTR is a common idiom in avr applications to put a string literal into flash
(progmem) memory.
Johann
