Re: [ping] Re: m32c: pointer math vs sizetype again
On Fri, Nov 14, 2008 at 10:23 AM, DJ Delorie <[EMAIL PROTECTED]> wrote: > >> > Now I'm getting a ton of errors (like, around 5000) that look like this: >> >> Just remove that assert for testing. > > Looks good without the assert, 21 new passes and only 1 new failure: > > PASS-FAIL: gcc.c-torture/execute/loop-ivopts-1.c execution, -O3 > -fomit-frame-pointer -funroll-loops Ok. I'll think of a "complete" patch and bootstrap, test and install it. Richard.
Re: [ping] Re: m32c: pointer math vs sizetype again
> > Now I'm getting a ton of errors (like, around 5000) that look like this: > > Just remove that assert for testing. Looks good without the assert, 21 new passes and only 1 new failure: PASS-FAIL: gcc.c-torture/execute/loop-ivopts-1.c execution, -O3 -fomit-frame-pointer -funroll-loops
Re: [ping] Re: m32c: pointer math vs sizetype again
On Thu, Nov 13, 2008 at 5:25 PM, DJ Delorie <[EMAIL PROTECTED]> wrote:
>
>> I don't think this is a suitable general solution. Can you instead try the
>> attached which again tries to simply make sure we sign-extend a sizetype
>> offset if that is smaller than the pointer mode.
>
> Now I'm getting a ton of errors (like, around 5000) that look like this:
Just remove that assert for testing.
Richard.
> [ gdb ] up
> #1 0x0854aca0 in build2_stat (code=PLUS_EXPR, tt=0xb7efc680,
> arg0=0xb7f76420, arg1=0xb7f717fc)
>at ../../gcc/gcc/tree.c:3293
> 3293gcc_assert (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1)
> == INTEGER_CST);
>
> [ gdb ] call debug_tree(arg0)
> type type size
>unit size
>align 8 symtab 0 alias set 2 canonical type 0xb7ef52d8 precision
> 16 min max
>pointer_to_this >
>unsigned PSI
>size
>unit size
>align 8 symtab 0 alias set -1 canonical type 0xb7efc680>
>constant
>arg 0
>used static ignored HI file
> /greed/dj/m32c/gcc/gcc/gcc/testsuite/gcc.dg/tree-ssa/predcom-1.c line 10 col
> 6 size unit size
>align 8
>(mem:HI (symbol_ref:PSI ("test_var.48") [flags 0x2] 0xb7f6c420 test_var.48>) [0 S2 A8])>>
>
> [ gdb ] call debug_tree(arg1)
> constant 2000>
>
> [ gdb ] where
> #0 fancy_abort (file=0x87b9369 "../../gcc/gcc/tree.c", line=3293,
>function=0x87b8e84 "build2_stat") at ../../gcc/gcc/diagnostic.c:712
> #1 0x0854aca0 in build2_stat (code=PLUS_EXPR, tt=0xb7efc680,
> arg0=0xb7f76420, arg1=0xb7f717fc)
>at ../../gcc/gcc/tree.c:3293
> #2 0x081dd315 in expand_expr_real_1 (exp=0xb7f734c8, target=0x0,
> tmode=PSImode,
>modifier=EXPAND_NORMAL, alt_rtl=0x0) at ../../gcc/gcc/expr.c:8330
> #3 0x081edc48 in expand_expr_real (exp=0xb7f734c8, target=0x0, tmode=PSImode,
>modifier=EXPAND_NORMAL, alt_rtl=0x0) at ../../gcc/gcc/expr.c:7121
> #4 0x08494789 in computation_cost (expr=0xb7f734c8, speed=0 '\0') at
> ../../gcc/gcc/expr.h:538
> #5 0x084975b7 in force_expr_to_var_cost (expr=0xb7f71674, speed=1 '\001')
>at ../../gcc/gcc/tree-ssa-loop-ivopts.c:3243
> #6 0x08497e15 in force_var_cost (data=0xbfffe458, expr=0xb7f71674,
> depends_on=0xbfffe35c)
>at ../../gcc/gcc/tree-ssa-loop-ivopts.c:3360
> #7 0x0849da6c in determine_use_iv_cost (data=0xbfffe458, use=0x887a1c8,
> cand=0x887a1f0)
>at ../../gcc/gcc/tree-ssa-loop-ivopts.c:3873
> #8 0x0849dbca in determine_use_iv_costs (data=0xbfffe458)
>at ../../gcc/gcc/tree-ssa-loop-ivopts.c:3960
> #9 0x084a0de1 in tree_ssa_iv_optimize () at
> ../../gcc/gcc/tree-ssa-loop-ivopts.c:5549
> #10 0x084ad965 in tree_ssa_loop_ivopts () at ../../gcc/gcc/tree-ssa-loop.c:667
> #11 0x0830363f in execute_one_pass (pass=0x8802240) at
> ../../gcc/gcc/passes.c:1279
> #12 0x083038ac in execute_pass_list (pass=0x8802240) at
> ../../gcc/gcc/passes.c:1328
> #13 0x083038bf in execute_pass_list (pass=0x8801e00) at
> ../../gcc/gcc/passes.c:1329
> #14 0x083038bf in execute_pass_list (pass=0x8801680) at
> ../../gcc/gcc/passes.c:1329
> #15 0x084215f0 in tree_rest_of_compilation (fndecl=0xb7f5e600) at
> ../../gcc/gcc/tree-optimize.c:418
> #16 0x0858ef37 in cgraph_expand_function (node=0xb7f5e680) at
> ../../gcc/gcc/cgraphunit.c:1038
> #17 0x08590d6f in cgraph_optimize () at ../../gcc/gcc/cgraphunit.c:1097
> #18 0x0805eb8b in c_write_global_declarations () at
> ../../gcc/gcc/c-decl.c:8074
> #19 0x083cbdf6 in toplev_main (argc=16, argv=0xbfffe714) at
> ../../gcc/gcc/toplev.c:979
> #20 0x080d6722 in main (argc=131140, argv=0x0) at ../../gcc/gcc/main.c:35
>
Re: [ping] Re: m32c: pointer math vs sizetype again
> I don't think this is a suitable general solution. Can you instead try the
> attached which again tries to simply make sure we sign-extend a sizetype
> offset if that is smaller than the pointer mode.
Now I'm getting a ton of errors (like, around 5000) that look like this:
[ gdb ] up
#1 0x0854aca0 in build2_stat (code=PLUS_EXPR, tt=0xb7efc680, arg0=0xb7f76420,
arg1=0xb7f717fc)
at ../../gcc/gcc/tree.c:3293
3293gcc_assert (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) ==
INTEGER_CST);
[ gdb ] call debug_tree(arg0)
unit size
align 8 symtab 0 alias set 2 canonical type 0xb7ef52d8 precision 16
min max
pointer_to_this >
unsigned PSI
size
unit size
align 8 symtab 0 alias set -1 canonical type 0xb7efc680>
constant
arg 0
used static ignored HI file
/greed/dj/m32c/gcc/gcc/gcc/testsuite/gcc.dg/tree-ssa/predcom-1.c line 10 col 6
size unit size
align 8
(mem:HI (symbol_ref:PSI ("test_var.48") [flags 0x2] ) [0 S2 A8])>>
[ gdb ] call debug_tree(arg1)
constant 2000>
[ gdb ] where
#0 fancy_abort (file=0x87b9369 "../../gcc/gcc/tree.c", line=3293,
function=0x87b8e84 "build2_stat") at ../../gcc/gcc/diagnostic.c:712
#1 0x0854aca0 in build2_stat (code=PLUS_EXPR, tt=0xb7efc680, arg0=0xb7f76420,
arg1=0xb7f717fc)
at ../../gcc/gcc/tree.c:3293
#2 0x081dd315 in expand_expr_real_1 (exp=0xb7f734c8, target=0x0,
tmode=PSImode,
modifier=EXPAND_NORMAL, alt_rtl=0x0) at ../../gcc/gcc/expr.c:8330
#3 0x081edc48 in expand_expr_real (exp=0xb7f734c8, target=0x0, tmode=PSImode,
modifier=EXPAND_NORMAL, alt_rtl=0x0) at ../../gcc/gcc/expr.c:7121
#4 0x08494789 in computation_cost (expr=0xb7f734c8, speed=0 '\0') at
../../gcc/gcc/expr.h:538
#5 0x084975b7 in force_expr_to_var_cost (expr=0xb7f71674, speed=1 '\001')
at ../../gcc/gcc/tree-ssa-loop-ivopts.c:3243
#6 0x08497e15 in force_var_cost (data=0xbfffe458, expr=0xb7f71674,
depends_on=0xbfffe35c)
at ../../gcc/gcc/tree-ssa-loop-ivopts.c:3360
#7 0x0849da6c in determine_use_iv_cost (data=0xbfffe458, use=0x887a1c8,
cand=0x887a1f0)
at ../../gcc/gcc/tree-ssa-loop-ivopts.c:3873
#8 0x0849dbca in determine_use_iv_costs (data=0xbfffe458)
at ../../gcc/gcc/tree-ssa-loop-ivopts.c:3960
#9 0x084a0de1 in tree_ssa_iv_optimize () at
../../gcc/gcc/tree-ssa-loop-ivopts.c:5549
#10 0x084ad965 in tree_ssa_loop_ivopts () at ../../gcc/gcc/tree-ssa-loop.c:667
#11 0x0830363f in execute_one_pass (pass=0x8802240) at
../../gcc/gcc/passes.c:1279
#12 0x083038ac in execute_pass_list (pass=0x8802240) at
../../gcc/gcc/passes.c:1328
#13 0x083038bf in execute_pass_list (pass=0x8801e00) at
../../gcc/gcc/passes.c:1329
#14 0x083038bf in execute_pass_list (pass=0x8801680) at
../../gcc/gcc/passes.c:1329
#15 0x084215f0 in tree_rest_of_compilation (fndecl=0xb7f5e600) at
../../gcc/gcc/tree-optimize.c:418
#16 0x0858ef37 in cgraph_expand_function (node=0xb7f5e680) at
../../gcc/gcc/cgraphunit.c:1038
#17 0x08590d6f in cgraph_optimize () at ../../gcc/gcc/cgraphunit.c:1097
#18 0x0805eb8b in c_write_global_declarations () at ../../gcc/gcc/c-decl.c:8074
#19 0x083cbdf6 in toplev_main (argc=16, argv=0xbfffe714) at
../../gcc/gcc/toplev.c:979
#20 0x080d6722 in main (argc=131140, argv=0x0) at ../../gcc/gcc/main.c:35
Re: [ping] Re: m32c: pointer math vs sizetype again
On Wed, Nov 12, 2008 at 10:40 AM, DJ Delorie <[EMAIL PROTECTED]> wrote:
>
> Ping? Is this the right thing to do? I'd like to get this (and a few
> other m32c bugs) resolved before the next release.
I don't think this is a suitable general solution. Can you instead try the
attached which again tries to simply make sure we sign-extend a sizetype
offset if that is smaller than the pointer mode.
Thanks,
Richard.
>> This seems to work, with a suitable extendhipsi2 pattern for m32c:
>>
>> Index: expr.c
>> ===
>> --- expr.c(revision 140759)
>> +++ expr.c(working copy)
>> @@ -8455,12 +8459,34 @@ expand_expr_real_1 (tree exp, rtx target
>> if (modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER)
>> op0 = force_operand (op0, target);
>> return REDUCE_BIT_FIELD (op0);
>> }
>> }
>>
>> + if (code == POINTER_PLUS_EXPR
>> + || TYPE_PRECISION (TREE_OPERAND (exp, 1)) < TYPE_PRECISION
>> (TREE_OPERAND (exp, 0)))
>> + {
>> + rtx reg;
>> + enum machine_mode imode, omode;
>> +
>> + expand_operands (TREE_OPERAND (exp, 0), TREE_OPERAND (exp, 1),
>> +subtarget, &op0, &op1, 0);
>> +
>> + if (op0 == const0_rtx)
>> + return op1;
>> + if (op1 == const0_rtx)
>> + return op0;
>> +
>> + imode = GET_MODE (op1);
>> + omode = GET_MODE (op0);
>> + reg = gen_reg_rtx (omode);
>> + emit_move_insn (reg, gen_rtx_fmt_e (SIGN_EXTEND, omode, op1));
>> + op1 = reg;
>> + goto binop2;
>> + }
>> +
>>/* No sense saving up arithmetic to be done
>>if it's all in the wrong mode to form part of an address.
>>And force_operand won't know whether to sign-extend or
>>zero-extend. */
>>if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER)
>> || mode != ptr_mode)
>>
>
o
Description: Binary data
[ping] Re: m32c: pointer math vs sizetype again
Ping? Is this the right thing to do? I'd like to get this (and a few
other m32c bugs) resolved before the next release.
> This seems to work, with a suitable extendhipsi2 pattern for m32c:
>
> Index: expr.c
> ===
> --- expr.c(revision 140759)
> +++ expr.c(working copy)
> @@ -8455,12 +8459,34 @@ expand_expr_real_1 (tree exp, rtx target
> if (modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER)
> op0 = force_operand (op0, target);
> return REDUCE_BIT_FIELD (op0);
> }
> }
>
> + if (code == POINTER_PLUS_EXPR
> + || TYPE_PRECISION (TREE_OPERAND (exp, 1)) < TYPE_PRECISION
> (TREE_OPERAND (exp, 0)))
> + {
> + rtx reg;
> + enum machine_mode imode, omode;
> +
> + expand_operands (TREE_OPERAND (exp, 0), TREE_OPERAND (exp, 1),
> +subtarget, &op0, &op1, 0);
> +
> + if (op0 == const0_rtx)
> + return op1;
> + if (op1 == const0_rtx)
> + return op0;
> +
> + imode = GET_MODE (op1);
> + omode = GET_MODE (op0);
> + reg = gen_reg_rtx (omode);
> + emit_move_insn (reg, gen_rtx_fmt_e (SIGN_EXTEND, omode, op1));
> + op1 = reg;
> + goto binop2;
> + }
> +
>/* No sense saving up arithmetic to be done
>if it's all in the wrong mode to form part of an address.
>And force_operand won't know whether to sign-extend or
>zero-extend. */
>if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER)
> || mode != ptr_mode)
>
Re: m32c: pointer math vs sizetype again
This seems to work, with a suitable extendhipsi2 pattern for m32c:
Index: expr.c
===
--- expr.c (revision 140759)
+++ expr.c (working copy)
@@ -8455,12 +8459,34 @@ expand_expr_real_1 (tree exp, rtx target
if (modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER)
op0 = force_operand (op0, target);
return REDUCE_BIT_FIELD (op0);
}
}
+ if (code == POINTER_PLUS_EXPR
+ || TYPE_PRECISION (TREE_OPERAND (exp, 1)) < TYPE_PRECISION
(TREE_OPERAND (exp, 0)))
+ {
+ rtx reg;
+ enum machine_mode imode, omode;
+
+ expand_operands (TREE_OPERAND (exp, 0), TREE_OPERAND (exp, 1),
+ subtarget, &op0, &op1, 0);
+
+ if (op0 == const0_rtx)
+ return op1;
+ if (op1 == const0_rtx)
+ return op0;
+
+ imode = GET_MODE (op1);
+ omode = GET_MODE (op0);
+ reg = gen_reg_rtx (omode);
+ emit_move_insn (reg, gen_rtx_fmt_e (SIGN_EXTEND, omode, op1));
+ op1 = reg;
+ goto binop2;
+ }
+
/* No sense saving up arithmetic to be done
if it's all in the wrong mode to form part of an address.
And force_operand won't know whether to sign-extend or
zero-extend. */
if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER)
|| mode != ptr_mode)
Re: m32c: pointer math vs sizetype again
> Ok, I just looked for POINTER_PLUS_EXPR and tried to make sure its > offset is always sign-extended. Can you check where on the call > path above we miss this special treatment? It looks like it drops past them all and gets the generic treatment, just before the MINUS_EXPR case.
Re: m32c: pointer math vs sizetype again
On Thu, Oct 2, 2008 at 8:50 PM, DJ Delorie <[EMAIL PROTECTED]> wrote: > > Ok, here's where it's getting expanded: > > #12 0x081e2805 in convert_modes (mode=PSImode, oldmode=HImode, x=0xb7f63450, > unsignedp=1) >at ../../gcc/gcc/expr.c:859 > #13 0x0831f83a in expand_binop_directly (mode=945, binoptab=0x8824de0, > op0=0xb7f63410, >op1=0xb7f63450, target=0xb7f63440, unsignedp=, > methods=OPTAB_LIB_WIDEN, >last=0xb7ef16c0) at ../../gcc/gcc/optabs.c:1488 > #14 0x0831aa62 in expand_binop (mode=PSImode, binoptab=0x8824de0, > op0=0xb7f63410, op1=0xb7f63450, >target=0xb7f63440, unsignedp=1, methods=OPTAB_LIB_WIDEN) at > ../../gcc/gcc/optabs.c:1601 > #15 0x0821cb22 in expand_expr_real_1 (exp=0xb7ef160c, target=0xb7f63440, > tmode=PSImode, >modifier=EXPAND_NORMAL, alt_rtl=0x0) at ../../gcc/gcc/expr.c:9452 > > At #15 the type of the target is used to decide if the math is signed > or unsigned, so adding a signed HI to an unsigned PSI results in a > zero extend. > > The patch you added was for the multiply-and-add case, such as "int *p > += 1" but it's skipped if there's no multiply (char *p). Ok, I just looked for POINTER_PLUS_EXPR and tried to make sure its offset is always sign-extended. Can you check where on the call path above we miss this special treatment? Thanks, Richard.
Re: m32c: pointer math vs sizetype again
Ok, here's where it's getting expanded: #12 0x081e2805 in convert_modes (mode=PSImode, oldmode=HImode, x=0xb7f63450, unsignedp=1) at ../../gcc/gcc/expr.c:859 #13 0x0831f83a in expand_binop_directly (mode=945, binoptab=0x8824de0, op0=0xb7f63410, op1=0xb7f63450, target=0xb7f63440, unsignedp=, methods=OPTAB_LIB_WIDEN, last=0xb7ef16c0) at ../../gcc/gcc/optabs.c:1488 #14 0x0831aa62 in expand_binop (mode=PSImode, binoptab=0x8824de0, op0=0xb7f63410, op1=0xb7f63450, target=0xb7f63440, unsignedp=1, methods=OPTAB_LIB_WIDEN) at ../../gcc/gcc/optabs.c:1601 #15 0x0821cb22 in expand_expr_real_1 (exp=0xb7ef160c, target=0xb7f63440, tmode=PSImode, modifier=EXPAND_NORMAL, alt_rtl=0x0) at ../../gcc/gcc/expr.c:9452 At #15 the type of the target is used to decide if the math is signed or unsigned, so adding a signed HI to an unsigned PSI results in a zero extend. The patch you added was for the multiply-and-add case, such as "int *p += 1" but it's skipped if there's no multiply (char *p).
Re: m32c: pointer math vs sizetype again
This is what I get with your patch and not mine:
neg.w r0 ; 7neghi2/1
mov.w r0,a0; 9
zero_extendhipsi2
add.l a0,r3r1 ; 15
addpsi3/3
Somewhere else, it's getting zero-extended when it should be
sign-extended. That's why I was trying to use ssizetype in c-common.c
Index: expr.c
===
--- expr.c (revision 140759)
+++ expr.c (working copy)
@@ -8338,14 +8338,18 @@ expand_expr_real_1 (tree exp, rtx target
== TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (subsubexp1, 0
&& (TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (subsubexp0, 0)))
== TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (subsubexp1, 0)
{
tree op0type = TREE_TYPE (TREE_OPERAND (subsubexp0, 0));
enum machine_mode innermode = TYPE_MODE (op0type);
- bool zextend_p = TYPE_UNSIGNED (op0type);
+ bool zextend_p;
bool sat_p = TYPE_SATURATING (TREE_TYPE (subsubexp0));
+ if (code == POINTER_PLUS_EXPR)
+ zextend_p = false;
+ else
+ zextend_p = TYPE_UNSIGNED (op0type);
if (sat_p == 0)
this_optab = zextend_p ? umadd_widen_optab : smadd_widen_optab;
else
this_optab = zextend_p ? usmadd_widen_optab
: ssmadd_widen_optab;
if (mode == GET_MODE_2XWIDER_MODE (innermode)
Re: m32c: pointer math vs sizetype again
> (what is the current sizetype mode?) HImode > There are no bitfield instructions. What I suggest is that you make > sizetype 16bits (HImode), as if I remember correctly you are not > concerned anyway about pointer offsets larger than what fits into > signed 16bits. If you then use the expansion patch I suggested I > don't see why it should not work. I'll poke at it some more, I thought I had your patch in there.
Re: m32c: pointer math vs sizetype again
On Thu, Oct 2, 2008 at 12:43 AM, DJ Delorie <[EMAIL PROTECTED]> wrote: > >> I think this is the wrong place to fix this. If you would override >> the sizetypes precision from your target, would that fix it? That >> is, in stor-layout.c set_sizetype make the target allow adjusting >> the passed type (which is supposed to be sizetype). If at all then >> these types should be consistent. > > The problem is that the chip has 24 bit pointers, but 16 bit > registers. It has math operations for 16 bit numbers and some 32 bit > numbers (the rest are emulated). It has a few operations for 24 bit > numbers. There are no C types for 24 bit numbers (PSImode is 32 bits > wide with 24 bit precision, if I tweak its precision manually it tries > to use bitfield instructions all over the place, if I don't it uses > "long int" which is wrong). (what is the current sizetype mode?) There are no bitfield instructions. What I suggest is that you make sizetype 16bits (HImode), as if I remember correctly you are not concerned anyway about pointer offsets larger than what fits into signed 16bits. If you then use the expansion patch I suggested I don't see why it should not work. Richard.
Re: m32c: pointer math vs sizetype again
> Can you look in the CVS/SVN archives and see what the mn102 port did -- It used SImode for size_type but I think I tried that and it blew up in useless_type_conversion_p. I can try again if you're interested in the details.
Re: m32c: pointer math vs sizetype again
DJ Delorie wrote: I think this is the wrong place to fix this. If you would override the sizetypes precision from your target, would that fix it? That is, in stor-layout.c set_sizetype make the target allow adjusting the passed type (which is supposed to be sizetype). If at all then these types should be consistent. The problem is that the chip has 24 bit pointers, but 16 bit registers. It has math operations for 16 bit numbers and some 32 bit numbers (the rest are emulated). It has a few operations for 24 bit numbers. There are no C types for 24 bit numbers (PSImode is 32 bits wide with 24 bit precision, if I tweak its precision manually it tries to use bitfield instructions all over the place, if I don't it uses "long int" which is wrong). All I want for now is to treat ptr+int as a signed addition, not an unsigned one. My patch is just trying to detect the case where a sign extension is needed at all, and insert it. Can you look in the CVS/SVN archives and see what the mn102 port did -- it had the same core properties as the chip you're describing. It was a 16/24 bit chip (true 24bit address registers), mostly 16bit ops with a few 24bit ops. All 32bit ops were synthesized. Jeff
Re: m32c: pointer math vs sizetype again
> I think this is the wrong place to fix this. If you would override > the sizetypes precision from your target, would that fix it? That > is, in stor-layout.c set_sizetype make the target allow adjusting > the passed type (which is supposed to be sizetype). If at all then > these types should be consistent. The problem is that the chip has 24 bit pointers, but 16 bit registers. It has math operations for 16 bit numbers and some 32 bit numbers (the rest are emulated). It has a few operations for 24 bit numbers. There are no C types for 24 bit numbers (PSImode is 32 bits wide with 24 bit precision, if I tweak its precision manually it tries to use bitfield instructions all over the place, if I don't it uses "long int" which is wrong). All I want for now is to treat ptr+int as a signed addition, not an unsigned one. My patch is just trying to detect the case where a sign extension is needed at all, and insert it.
Re: m32c: pointer math vs sizetype again
On Wed, Oct 1, 2008 at 12:20 AM, DJ Delorie <[EMAIL PROTECTED]> wrote:
>
> I've got a partial patch which works with older (4.3) gccs, but fails
> gimple's check for trunk (attached). My trivial test case...
>
> char *
> foo (char *a, int b)
> {
> return a-b;
> }
>
> ...fails thusly:
>
> size
> constant 32>
>unit size int> constant 4>
>align 8 symtab 0 alias set -1 canonical type 0xb7f52c30 precision 32 min
> max >
> size
> constant 16>
>unit size int> constant 2>
>align 8 symtab 0 alias set -1 canonical type 0xb7efc000 precision 16 min
> max >
> useless false: ../../gcc/gcc/tree-ssa.c 1092
> dj.c: In function 'foo':
> dj.c:2: error: type mismatch in pointer plus expression
> D.1194 = a + D.1196;
>
> char *
>
> char *
>
>
>
> D.1194 = a + D.1196;
>
> dj.c:2: internal compiler error: verify_gimple failed
>
>
> I'm obviously doing something wrong in the cast-to-bigger step. How
> can I get this to pass gimple? What I'm trying to accomplish is this:
>
> 1. Values added to pointers need to be treated as signed (at least, if
> they're signed types, certainly if you're going to use a
> NEGATE_EXPR).
>
> 2. If sizeof(size_t) < sizeof(void *), sign extend the intop to be
> pointer-sized before adding it.
>
>
>
> Index: c-common.c
> ===
> --- c-common.c (revision 140759)
> +++ c-common.c (working copy)
> @@ -3337,20 +3337,28 @@ pointer_int_sum (enum tree_code resultco
> intop = convert (c_common_type_for_size (TYPE_PRECISION (sizetype),
> TYPE_UNSIGNED (sizetype)), intop);
>
> /* Replace the integer argument with a suitable product by the object size.
> Do this multiplication as signed, then convert to the appropriate
> type for the pointer operation. */
> - intop = convert (sizetype,
> + intop = convert (ssizetype,
> build_binary_op (EXPR_LOCATION (intop),
>MULT_EXPR, intop,
>convert (TREE_TYPE (intop), size_exp), 1));
>
> /* Create the sum or difference. */
> if (resultcode == MINUS_EXPR)
> -intop = fold_build1 (NEGATE_EXPR, sizetype, intop);
> +intop = fold_build1 (NEGATE_EXPR, ssizetype, intop);
> +
> + if (TREE_CODE (result_type) == POINTER_TYPE
> + && TYPE_PRECISION (result_type) > TYPE_PRECISION (TREE_TYPE (intop)))
> +{
> + tree iptr_type = c_common_type_for_mode (TYPE_MODE (result_type),
> + TYPE_UNSIGNED (result_type));
> + intop = fold_build1 (NOP_EXPR, iptr_type, intop);
> +}
>
> ret = fold_build2 (POINTER_PLUS_EXPR, result_type, ptrop, intop);
>
> fold_undefer_and_ignore_overflow_warnings ();
>
> return ret;
I think this is the wrong place to fix this. If you would override
the sizetypes precision
from your target, would that fix it? That is, in stor-layout.c
set_sizetype make the
target allow adjusting the passed type (which is supposed to be
sizetype). If at all
then these types should be consistent.
Richard.
Re: m32c: pointer math vs sizetype again
> Is this related to the loop termination bug I reported > on the m32c? > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37665 Probably related, but I don't know if a patch to fix one will fix the other.
Re: m32c: pointer math vs sizetype again
Is this related to the loop termination bug I reported
on the m32c?
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37665
The generated code is using the lower 16-bits of the
address for end of the array rather than the full 24-bit
address.
--joel
DJ Delorie wrote:
I've got a partial patch which works with older (4.3) gccs, but fails
gimple's check for trunk (attached). My trivial test case...
char *
foo (char *a, int b)
{
return a-b;
}
...fails thusly:
constant 32>
unit size
constant 4>
align 8 symtab 0 alias set -1 canonical type 0xb7f52c30 precision 32 min max >
constant 16>
unit size
constant 2>
align 8 symtab 0 alias set -1 canonical type 0xb7efc000 precision 16 min max >
useless false: ../../gcc/gcc/tree-ssa.c 1092
dj.c: In function 'foo':
dj.c:2: error: type mismatch in pointer plus expression
D.1194 = a + D.1196;
char *
char *
D.1194 = a + D.1196;
dj.c:2: internal compiler error: verify_gimple failed
I'm obviously doing something wrong in the cast-to-bigger step. How
can I get this to pass gimple? What I'm trying to accomplish is this:
1. Values added to pointers need to be treated as signed (at least, if
they're signed types, certainly if you're going to use a
NEGATE_EXPR).
2. If sizeof(size_t) < sizeof(void *), sign extend the intop to be
pointer-sized before adding it.
Index: c-common.c
===
--- c-common.c (revision 140759)
+++ c-common.c (working copy)
@@ -3337,20 +3337,28 @@ pointer_int_sum (enum tree_code resultco
intop = convert (c_common_type_for_size (TYPE_PRECISION (sizetype),
TYPE_UNSIGNED (sizetype)), intop);
/* Replace the integer argument with a suitable product by the object size.
Do this multiplication as signed, then convert to the appropriate
type for the pointer operation. */
- intop = convert (sizetype,
+ intop = convert (ssizetype,
build_binary_op (EXPR_LOCATION (intop),
MULT_EXPR, intop,
convert (TREE_TYPE (intop), size_exp), 1));
/* Create the sum or difference. */
if (resultcode == MINUS_EXPR)
-intop = fold_build1 (NEGATE_EXPR, sizetype, intop);
+intop = fold_build1 (NEGATE_EXPR, ssizetype, intop);
+
+ if (TREE_CODE (result_type) == POINTER_TYPE
+ && TYPE_PRECISION (result_type) > TYPE_PRECISION (TREE_TYPE (intop)))
+{
+ tree iptr_type = c_common_type_for_mode (TYPE_MODE (result_type),
+ TYPE_UNSIGNED (result_type));
+ intop = fold_build1 (NOP_EXPR, iptr_type, intop);
+}
ret = fold_build2 (POINTER_PLUS_EXPR, result_type, ptrop, intop);
fold_undefer_and_ignore_overflow_warnings ();
return ret;
Index: tree.c
===
--- tree.c (revision 140759)
+++ tree.c (working copy)
@@ -3283,15 +3283,21 @@ build2_stat (enum tree_code code, tree t
if ((code == MINUS_EXPR || code == PLUS_EXPR || code == MULT_EXPR)
&& arg0 && arg1 && tt && POINTER_TYPE_P (tt))
gcc_assert (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) ==
INTEGER_CST);
if (code == POINTER_PLUS_EXPR && arg0 && arg1 && tt)
-gcc_assert (POINTER_TYPE_P (tt) && POINTER_TYPE_P (TREE_TYPE (arg0))
- && INTEGRAL_TYPE_P (TREE_TYPE (arg1))
- && useless_type_conversion_p (sizetype, TREE_TYPE (arg1)));
+{
+ gcc_assert (POINTER_TYPE_P (tt));
+ gcc_assert (POINTER_TYPE_P (TREE_TYPE (arg0)));
+ gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (arg1)));
+#if 0
+ gcc_assert (useless_type_conversion_p (sizetype, TREE_TYPE (arg1))
+ || useless_type_conversion_p (ssizetype, TREE_TYPE (arg1)));
+#endif
+}
t = make_node_stat (code PASS_MEM_STAT);
TREE_TYPE (t) = tt;
/* Below, we automatically set TREE_SIDE_EFFECTS and TREE_READONLY for the
result based on those same flags for the arguments. But if the
--
Joel Sherrill, Ph.D. Director of Research & Development
[EMAIL PROTECTED]On-Line Applications Research
Ask me about RTEMS: a free RTOS Huntsville AL 35805
Support Available (256) 722-9985
Re: m32c: pointer math vs sizetype again
I've got a partial patch which works with older (4.3) gccs, but fails
gimple's check for trunk (attached). My trivial test case...
char *
foo (char *a, int b)
{
return a-b;
}
...fails thusly:
constant 32>
unit size constant 4>
align 8 symtab 0 alias set -1 canonical type 0xb7f52c30 precision 32 min
max >
constant 16>
unit size constant 2>
align 8 symtab 0 alias set -1 canonical type 0xb7efc000 precision 16 min
max >
useless false: ../../gcc/gcc/tree-ssa.c 1092
dj.c: In function 'foo':
dj.c:2: error: type mismatch in pointer plus expression
D.1194 = a + D.1196;
char *
char *
D.1194 = a + D.1196;
dj.c:2: internal compiler error: verify_gimple failed
I'm obviously doing something wrong in the cast-to-bigger step. How
can I get this to pass gimple? What I'm trying to accomplish is this:
1. Values added to pointers need to be treated as signed (at least, if
they're signed types, certainly if you're going to use a
NEGATE_EXPR).
2. If sizeof(size_t) < sizeof(void *), sign extend the intop to be
pointer-sized before adding it.
Index: c-common.c
===
--- c-common.c (revision 140759)
+++ c-common.c (working copy)
@@ -3337,20 +3337,28 @@ pointer_int_sum (enum tree_code resultco
intop = convert (c_common_type_for_size (TYPE_PRECISION (sizetype),
TYPE_UNSIGNED (sizetype)), intop);
/* Replace the integer argument with a suitable product by the object size.
Do this multiplication as signed, then convert to the appropriate
type for the pointer operation. */
- intop = convert (sizetype,
+ intop = convert (ssizetype,
build_binary_op (EXPR_LOCATION (intop),
MULT_EXPR, intop,
convert (TREE_TYPE (intop), size_exp), 1));
/* Create the sum or difference. */
if (resultcode == MINUS_EXPR)
-intop = fold_build1 (NEGATE_EXPR, sizetype, intop);
+intop = fold_build1 (NEGATE_EXPR, ssizetype, intop);
+
+ if (TREE_CODE (result_type) == POINTER_TYPE
+ && TYPE_PRECISION (result_type) > TYPE_PRECISION (TREE_TYPE (intop)))
+{
+ tree iptr_type = c_common_type_for_mode (TYPE_MODE (result_type),
+ TYPE_UNSIGNED (result_type));
+ intop = fold_build1 (NOP_EXPR, iptr_type, intop);
+}
ret = fold_build2 (POINTER_PLUS_EXPR, result_type, ptrop, intop);
fold_undefer_and_ignore_overflow_warnings ();
return ret;
Index: tree.c
===
--- tree.c (revision 140759)
+++ tree.c (working copy)
@@ -3283,15 +3283,21 @@ build2_stat (enum tree_code code, tree t
if ((code == MINUS_EXPR || code == PLUS_EXPR || code == MULT_EXPR)
&& arg0 && arg1 && tt && POINTER_TYPE_P (tt))
gcc_assert (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) ==
INTEGER_CST);
if (code == POINTER_PLUS_EXPR && arg0 && arg1 && tt)
-gcc_assert (POINTER_TYPE_P (tt) && POINTER_TYPE_P (TREE_TYPE (arg0))
- && INTEGRAL_TYPE_P (TREE_TYPE (arg1))
- && useless_type_conversion_p (sizetype, TREE_TYPE (arg1)));
+{
+ gcc_assert (POINTER_TYPE_P (tt));
+ gcc_assert (POINTER_TYPE_P (TREE_TYPE (arg0)));
+ gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (arg1)));
+#if 0
+ gcc_assert (useless_type_conversion_p (sizetype, TREE_TYPE (arg1))
+ || useless_type_conversion_p (ssizetype, TREE_TYPE (arg1)));
+#endif
+}
t = make_node_stat (code PASS_MEM_STAT);
TREE_TYPE (t) = tt;
/* Below, we automatically set TREE_SIDE_EFFECTS and TREE_READONLY for the
result based on those same flags for the arguments. But if the
Re: m32c: pointer math vs sizetype again
> NEGATE_EXPR on an unsigned type is fully defined. It's what you > should get when you say "unsigned int i, j; ...; i = - j;". I didn't say it was undefined, I said it seemed wrong. Esp since the example starts with a plain "int" value. > I think the problem you are facing may be that POINTER_PLUS_EXPR > generally assumes that the right type to use to add to a pointer is > sizetype. You may need to make it use a different type, though I'm > not sure what that type should be in your case. ptrdiff_type ? That is the "difference between pointers" type, which seems like *exactly* what you should use for adding/subtracting integer values to/from pointers.
Re: m32c: pointer math vs sizetype again
DJ Delorie <[EMAIL PROTECTED]> writes: >> Does the following fix it? > > Nope, sorry. I was looking at this code in c-common.c, where the expr > is first created, but I don't know what that ends up calling: > > /* Create the sum or difference. */ > if (resultcode == MINUS_EXPR) > intop = fold_build1 (NEGATE_EXPR, sizetype, intop); > > ret = fold_build2 (POINTER_PLUS_EXPR, result_type, ptrop, intop); > > It's calling NEGATE on an unsigned type, which seems wrong to me. NEGATE_EXPR on an unsigned type is fully defined. It's what you should get when you say "unsigned int i, j; ...; i = - j;". I think the problem you are facing may be that POINTER_PLUS_EXPR generally assumes that the right type to use to add to a pointer is sizetype. You may need to make it use a different type, though I'm not sure what that type should be in your case. Ian
Re: m32c: pointer math vs sizetype again
> Does the following fix it? Nope, sorry. I was looking at this code in c-common.c, where the expr is first created, but I don't know what that ends up calling: /* Create the sum or difference. */ if (resultcode == MINUS_EXPR) intop = fold_build1 (NEGATE_EXPR, sizetype, intop); ret = fold_build2 (POINTER_PLUS_EXPR, result_type, ptrop, intop); It's calling NEGATE on an unsigned type, which seems wrong to me.
Re: m32c: pointer math vs sizetype again
On Sat, Sep 20, 2008 at 1:52 AM, DJ Delorie <[EMAIL PROTECTED]> wrote:
>
> m32c-elf-gcc -mcpu=m32c (16 bit ints, 24 bit pointers) miscompiles
> this:
>
> int *foo (int *a, int b)
> {
> return a-b;
> }
>
> as this:
>
> _foo:
>enter #0 ; 30prologue_enter_24
>pushm r1,r3,a0; 31pushm
>; end of prologue ; 32prologue_end
>mov.w 12[fb],r0 ; 27movhi_op/1
>sha.w #1,r0 ; 7 ashlhi3_i/1
>neg.w r0 ; 8 neghi2/1
>mov.w r0,a0 ; 10zero_extendhipsi2
>mov.l 8[fb],r3r1 ; 28movpsi_op/2
>add.l a0,r3r1 ; 16addpsi3/3
>mov.l r3r1,mem0 ; 29movpsi_op/2
>; start of epilogue ; 35epilogue_start
>popmr1,r3,a0; 36popm
>exitd ; 37epilogue_exitd_24
>
> The key instructions are - neg, zero_extend, add. This breaks if the
> original value is, say, 2. Neg gives 0xfffe, zero_extend gives
> 0x00fffe, and you end up adding 65534 to the pointer.
>
> If I change sizetype to "long unsigned int", it's bigger than a
> pointer, and the front end leaves an unexpected nop_convert in various
> expressions, which causes ICEs.
>
> There is no standard integer type the same size as pointers (24 bit,
> PSImode). IIRC, last time I tried to shoehorn in a PSImode sizetype,
> gcc wanted a full set of math operators for it, which the chip doesn't
> have (they have to be done in HI or SI mode)
>
> I tried making sizetype signed, that ICEd too.
>
> What's the right thing to do?
>
> If the front end either sign extended, or had a POINTER_MINUS_EXPR,
> things would just work.
I think this is one of the reasons sizetype constants(!!) are always
sign-extended.
Of course this makes less sense if non-constants are not ;)
Does the following fix it?
Index: expr.c
===
--- expr.c (revision 140433)
+++ expr.c (working copy)
@@ -8336,8 +8336,12 @@ expand_expr_real_1 (tree exp, rtx target
{
tree op0type = TREE_TYPE (TREE_OPERAND (subsubexp0, 0));
enum machine_mode innermode = TYPE_MODE (op0type);
- bool zextend_p = TYPE_UNSIGNED (op0type);
+ bool zextend_p;
bool sat_p = TYPE_SATURATING (TREE_TYPE (subsubexp0));
+ if (code == POINTER_PLUS_EXPR)
+ zextend_p = false;
+ else
+ zextend_p = TYPE_UNSIGNED (op0type)
if (sat_p == 0)
this_optab = zextend_p ? umadd_widen_optab : smadd_widen_optab;
else
This would affect only targets where the precision of sizetype is not equal to
that of a pointer where it would now sign-extend instead of zero-extending for
C and sign-extending for Ada.
Richard.
> DJ
>
