Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Sun, Jul 10, 2011 at 4:51 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sun, Jul 10, 2011 at 2:04 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Sun, Jul 10, 2011 at 9:36 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>> On Sat, Jul 9, 2011 at 23:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Sat, Jul 9, 2011 at 2:18 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>>> On 07/05/2011 04:27 PM, H.J. Lu wrote:
>>>>>>>
>>>>>>> ?diff --git a/gcc/explow.c b/gcc/explow.c
>>>>>>> ?index 7387dad..b343bf8 100644
>>>>>>> ?--- a/gcc/explow.c
>>>>>>> ?+++ b/gcc/explow.c
>>>>>>> ?@@ -383,18 +383,13 @@ convert_memory_address_addr_space (enum
>>>>>>> machine_mode to_mode ATTRIBUTE_UNUSED,
>>>>>>>
>>>>>>> ? ? ?case PLUS:
>>>>>>> ? ? ?case MULT:
>>>>>>> ?- ? ? ?/* For addition we can safely permute the conversion and addition
>>>>>>> ?- ? ? ? ?operation if one operand is a constant and converting the
>>>>>>> constant
>>>>>>> ?- ? ? ? ?does not change it or if one operand is a constant and we are
>>>>>>> ?- ? ? ? ?using a ptr_extend instruction ?(POINTERS_EXTEND_UNSIGNED< ?0).
>>>>>>> ?- ? ? ? ?We can always safely permute them if we are making the address
>>>>>>> ?- ? ? ? ?narrower. ?*/
>>>>>>> ?+ ? ? ?/* For addition we safely permute the conversion and addition
>>>>>>> ?+ ? ? ? ?operation if one operand is a constant since we can't generate
>>>>>>> ?+ ? ? ? ?new instructions. ?We can always safely permute them if we are
>>>>>>> ?+ ? ? ? ?making the address narrower. ?*/
>>>>>>> ? ? ? ?if (GET_MODE_SIZE (to_mode)< ?GET_MODE_SIZE (from_mode)
>>>>>>> ? ? ? ? ? || (GET_CODE (x) == PLUS
>>>>>>> ?-&& ?CONST_INT_P (XEXP (x, 1))
>>>>>>> ?-&& ?(XEXP (x, 1) == convert_memory_address_addr_space
>>>>>>> ?- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?(to_mode, XEXP (x, 1), as)
>>>>>>> ?- ? ? ? ? ? ? ? ? || POINTERS_EXTEND_UNSIGNED< ?0)))
>>>>>>> ?+&& ?CONST_INT_P (XEXP (x, 1))))
>>>>>>> ? ? ? ? return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>>>>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?convert_memory_address_addr_space
>>>>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?(to_mode, XEXP (x, 0), as),
>>>>>
>>>>> This does not seem safe to me.
>>>>
>>>> The current code is broken for x32. ?See:
>>>>
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47727
>>>>
>>>> We can't generate any new instructions. ?Do you have any suggestions.
>>>
>>> By "safe" I mean that the new condition might be too wide and generate
>>> wrong code. ?Richard is definitely right in comment 6, generating new
>>> code in simplify-rtx is a no-no (see its usage of
>>> gen_lowpart_no_emit).
>>
>> Here is a different approach. ?I added convert_memory_address_addr_space_1
>> and convert_modes_1 so that simplify-rtx won't generate new insns. ?OK
>> for trunk if there are no regressions on Linux/x86?
>>
>> Thanks.
>>
>> --
>> H.J.
>> ---
>> 2011-07-10 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>
>> ? ? ? ?* explow.c (convert_memory_address_addr_space_1): New.
>> ? ? ? ?(convert_memory_address_addr_space): Use it.
>>
>> ? ? ? ?* expr.c (convert_modes_1): New.
>> ? ? ? ?(convert_modes): Use it.
>>
>> ? ? ? ?* expr.h (convert_modes_1): New.
>>
>> ? ? ? ?* rtl.h (convert_memory_address_addr_space_1): New.
>> ? ? ? ?(convert_memory_address_1): Likewise.
>>
>> ? ? ? ?* simplify-rtx.c (simplify_unary_operation_1): Call
>> ? ? ? ?convert_memory_address_1 instead of convert_memory_address.
>>
>
> It doesn't work. ?I got
>
> (gdb) f 2
> #2 ?0x000000000078735a in convert_memory_address_addr_space_1 (to_mode=DImode,
> ? ?x=0x7ffff05ac4e0, as=0 '\000', no_emit=1 '\001')
> ? ?at /export/gnu/import/git/gcc-x32/gcc/explow.c:410
> 410 ? ? ? return convert_modes_1 (to_mode, from_mode, x,
> (gdb) call debug_rtx (x)
> (plus:SI (symbol_ref:SI ("iplane.1577") [flags 0x2] <var_decl
> 0x7ffff0857960 iplane>)
> ? ?(const_int -4 [0xfffffffffffffffc]))
> (gdb) bt
> #0 ?fancy_abort (file=0x13531a8 "/export/gnu/import/git/gcc-x32/gcc/expr.c",
> ? ?line=798, function=0x1354a00 "convert_modes_1")
> ? ?at /export/gnu/import/git/gcc-x32/gcc/diagnostic.c:893
> #1 ?0x000000000079c60c in convert_modes_1 (mode=DImode, oldmode=SImode,
> ? ?x=0x7ffff05ac4e0, unsignedp=1, no_emit=1 '\001')
> ? ?at /export/gnu/import/git/gcc-x32/gcc/expr.c:798
> #2 ?0x000000000078735a in convert_memory_address_addr_space_1 (to_mode=DImode,
> ? ?x=0x7ffff05ac4e0, as=0 '\000', no_emit=1 '\001')
> ? ?at /export/gnu/import/git/gcc-x32/gcc/explow.c:410
> #3 ?0x0000000000787281 in convert_memory_address_addr_space_1 (to_mode=DImode,
> ? ?x=0x7ffff05616d0, as=0 '\000', no_emit=1 '\001')
> ? ?at /export/gnu/import/git/gcc-x32/gcc/explow.c:381
> #4 ?0x0000000000b0faf4 in simplify_unary_operation_1 (code=ZERO_EXTEND,
> ? ?mode=DImode, op=0x7ffff05616d0)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:1246
> #5 ?0x0000000000b0d851 in simplify_unary_operation (code=ZERO_EXTEND,
> ? ?mode=DImode, op=0x7ffff05616d0, op_mode=SImode)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:582
> #6 ?0x0000000000b0d035 in simplify_gen_unary (code=ZERO_EXTEND, mode=DImode,
> ? ?op=0x7ffff05616d0, op_mode=SImode)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:370
> #7 ?0x000000000117078a in if_then_else_cond (x=0x7ffff02ebb00,
> ? ?ptrue=0x7fffffffd720, pfalse=0x7fffffffd718)
> ---Type <return> to continue, or q <return> to quit---
> ? ?at /export/gnu/import/git/gcc-x32/gcc/combine.c:8649
> #8 ?0x00000000011675b5 in combine_simplify_rtx (x=0x7ffff02ebb00,
> ? ?op0_mode=SImode, in_dest=0, in_cond=0)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/combine.c:5316
> #9 ?0x0000000001167315 in subst (x=0x7ffff02ebb00, from=0x7ffff02f8120,
> ? ?to=0x7ffff05ac4f8, in_dest=0, in_cond=0, unique_copy=0)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/combine.c:5253
> #10 0x0000000001167104 in subst (x=0x7ffff02f5558, from=0x7ffff02f8120,
> ? ?to=0x7ffff05ac4f8, in_dest=0, in_cond=0, unique_copy=0)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/combine.c:5189
> #11 0x00000000011611ae in try_combine (i3=0x7ffff02f4c60, i2=0x7ffff02f4c18,
> ? ?i1=0x0, i0=0x0, new_direct_jump_p=0x7fffffffde14,
> ? ?last_combined_insn=0x7ffff02f4c60)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/combine.c:3178
> #12 0x000000000115c487 in combine_instructions (f=0x7ffff07e7700, nregs=3344)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/combine.c:1223
> #13 0x000000000117c64e in rest_of_handle_combine ()
> ? ?at /export/gnu/import/git/gcc-x32/gcc/combine.c:13879
> #14 0x0000000000a500e7 in execute_one_pass (pass=0x190d320)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/passes.c:2062
> #15 0x0000000000a502cd in execute_pass_list (pass=0x190d320)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/passes.c:2117
> #16 0x0000000000a502ee in execute_pass_list (pass=0x1908180)
> ---Type <return> to continue, or q <return> to quit---
> ? ?at /export/gnu/import/git/gcc-x32/gcc/passes.c:2118
> #17 0x0000000000be93e0 in tree_rest_of_compilation (fndecl=0x7ffff074c200)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/tree-optimize.c:416
> #18 0x00000000006d3ff7 in cgraph_expand_function (node=0x7ffff0792000)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:1804
> #19 0x00000000006d41b6 in cgraph_expand_all_functions ()
> ? ?at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:1863
> #20 0x00000000006d48b2 in cgraph_optimize ()
> ? ?at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:2133
> #21 0x00000000006d1b2a in cgraph_finalize_compilation_unit ()
> ? ?at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:1304
> #22 0x00000000009cb7b0 in write_global_declarations ()
> ? ?at /export/gnu/import/git/gcc-x32/gcc/langhooks.c:303
> #23 0x0000000000559cac in gfc_write_global_declarations ()
> ? ?at /export/gnu/import/git/gcc-x32/gcc/fortran/f95-lang.c:322
> #24 0x0000000000b4649c in compile_file ()
> ? ?at /export/gnu/import/git/gcc-x32/gcc/toplev.c:564
> #25 0x0000000000b48686 in do_compile ()
> ? ?at /export/gnu/import/git/gcc-x32/gcc/toplev.c:1886
> #26 0x0000000000b487ec in toplev_main (argc=19, argv=0x7fffffffe2a8)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/toplev.c:1958
> #27 0x000000000060be84 in main (argc=19, argv=0x7fffffffe2a8)
> ? ?at /export/gnu/import/git/gcc-x32/gcc/main.c:36
> (gdb)
>
>
> H.J.
>

With my original change,  I got

(const:DI (plus:DI (symbol_ref:DI ("iplane.1577") [flags 0x2]
<var_decl 0x7ffff0857960 iplane>)
        (const_int -4 [0xfffffffffffffffc])))

I think it is safe to permute the conversion and addition operation
if one operand is a constant and we are zero-extending.  This is
how zero-extending works.

-- 
H.J.