On Thu, Oct 2, 2014 at 10:13 PM, Vladimir Makarov <vmaka...@redhat.com> wrote:
>>>> I guess we achieved the consensus about the following patch to fix >>>> PR61360 >>>> >>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61360 >>>> >>>> The patch was successfully bootstrapped and tested (w/wo >>>> -march=amdfam10) on x86/x86-64. >>>> >>>> Is it ok to commit to trunk? >>> >>> >>> I've tested your patch and unfortunately it doesn't work: >>> >>> In file included from >>> /var/tmp/moz-build-dir/js/src/shell/Unified_cpp_js_src_shell0.cpp:15:0: >>> /var/tmp/mozilla-central/js/src/shell/js.cpp: In function ‘void >>> Process(JSContext*, JSObject*, const char*, bool)’: >>> /var/tmp/mozilla-central/js/src/shell/js.cpp:592:1: internal compiler >>> error: in lra_update_insn_recog_data, at lra.c:1221 >>> } >>> ^ >>> 0xa9d9ec lra_update_insn_recog_data(rtx_insn*) >>> ../../gcc/gcc/lra.c:1220 >>> 0xab450f eliminate_regs_in_insn >>> ../../gcc/gcc/lra-eliminations.c:1077 >>> 0xab450f process_insn_for_elimination >>> ../../gcc/gcc/lra-eliminations.c:1344 >>> 0xab450f lra_eliminate(bool, bool) >>> ../../gcc/gcc/lra-eliminations.c:1408 >>> 0xa9f2da lra(_IO_FILE*) >>> ../../gcc/gcc/lra.c:2270 >>> 0xa5d659 do_reload >>> ../../gcc/gcc/ira.c:5311 >>> 0xa5d659 execute >>> ../../gcc/gcc/ira.c:5470 >> >> >> Testcase is attached: >> >> % g++ -c -march=amdfam10 -w -O2 js.ii >> js.ii: In function ‘void RunFile(C)’: >> js.ii:64:1: internal compiler error: in lra_update_insn_recog_data, at >> lra.c:1221 >> > > Thanks for reporting this, Marcus. The problem now is in > optimize_function_for_size_p. It is false, when we define and cache enable > attributes at early stages (instantation of virtual regs) and true later. > > It is returning us to the same problem. I believe that we should not have > enable attributes depending on optimization options or on the current > running pass if we don't want the current solution in the trunk (with > recog_init). Setting right value for optimize_function_for_size_p does not > solve the problem as we can have different options for different functions > in the same compilation file. > > So minimal solution would be removing optimize_function_for_size_p from the > attribute definition. But I guess we could remove all condition. > Unfortunately, Ganesh did not post is it really beneficial to switch off > this alternative for AMD CPUs even if AMD optimization guide recommends it. I propose to split the pattern into size-optimized and normal pattern. The patch implements this proposal. Uros.
Index: i386.md =================================================================== --- i386.md (revision 215797) +++ i386.md (working copy) @@ -4766,6 +4766,38 @@ } }) +(define_insn "*float<SWI48:mode><MODEF:mode>2_sse_size" + [(set (match_operand:MODEF 0 "register_operand" "=f,x,x") + (float:MODEF + (match_operand:SWI48 1 "nonimmediate_operand" "m,r,m")))] + "SSE_FLOAT_MODE_P (<MODEF:MODE>mode) && TARGET_SSE_MATH + && optimize_function_for_size_p (cfun)" + "@ + fild%Z1\t%1 + %vcvtsi2<MODEF:ssemodesuffix><SWI48:rex64suffix>\t{%1, %d0|%d0, %1} + %vcvtsi2<MODEF:ssemodesuffix><SWI48:rex64suffix>\t{%1, %d0|%d0, %1}" + [(set_attr "type" "fmov,sseicvt,sseicvt") + (set_attr "prefix" "orig,maybe_vex,maybe_vex") + (set_attr "mode" "<MODEF:MODE>") + (set (attr "prefix_rex") + (if_then_else + (and (eq_attr "prefix" "maybe_vex") + (match_test "<SWI48:MODE>mode == DImode")) + (const_string "1") + (const_string "*"))) + (set_attr "unit" "i387,*,*") + (set_attr "athlon_decode" "*,double,direct") + (set_attr "amdfam10_decode" "*,vector,double") + (set_attr "bdver1_decode" "*,double,direct") + (set_attr "fp_int_src" "true") + (set (attr "enabled") + (cond [(eq_attr "alternative" "0") + (symbol_ref "TARGET_MIX_SSE_I387 + && X87_ENABLE_FLOAT (<MODEF:MODE>mode, + <SWI48:MODE>mode)") + ] + (const_int 1)))]) + (define_insn "*float<SWI48:mode><MODEF:mode>2_sse" [(set (match_operand:MODEF 0 "register_operand" "=f,x,x") (float:MODEF @@ -4795,16 +4827,9 @@ && X87_ENABLE_FLOAT (<MODEF:MODE>mode, <SWI48:MODE>mode)") (eq_attr "alternative" "1") - /* ??? For sched1 we need constrain_operands to be able to - select an alternative. Leave this enabled before RA. */ - (symbol_ref "TARGET_INTER_UNIT_CONVERSIONS - || optimize_function_for_size_p (cfun) - || !(reload_completed - || reload_in_progress - || lra_in_progress)") + (symbol_ref "TARGET_INTER_UNIT_CONVERSIONS") ] - (symbol_ref "true"))) - ]) + (const_int 1)))]) (define_insn "*float<SWI48x:mode><MODEF:mode>2_i387" [(set (match_operand:MODEF 0 "register_operand" "=f")