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")
