On Sat, 2015-01-17 at 22:40 +0900, Kaz Kojima wrote: > Oleg Endo <oleg.e...@t-online.de> wrote: > > Kaz, could you please test the patch on your sh4-linux setup and report > > your findings? Even though it's a bit late, I'd like to get this in for > > GCC 5, if it doesn't break too many things. > > Looks wrong code problem for tls and atomic constructs. > Here is the result of compare_tests for unpatched/patched > sh4-unknown-linux-gnu compilers: > > New tests that FAIL: > > ...
Thanks. Doesn't look so bad actually. I've expected worse. These are the two problems: 1) sh_find_extending_set_of_reg (introduced earlier as part of PR 53988) hits atomic insns, which in fact do a indirect sign extending mem load. The cmpeqsi_t splitter for const_int 0 then tries to use the value as sign extended, which wrongly converts an atomic into a simple mem load. The easy solution is not to report 'sign extended' in sh_find_extending_set_of_reg for mems that are buried in UNSPEC/UNSPECV insns. This also revealed a problem of inconsistent return values of sh_find_set_of_reg. This should be fixed regardless of the treg_set_expr stuff first. I'll create separate patch for that. The more complex solution would be to somehow convert the atomics so that the sign extension becomes visible for the following code. Maybe later. 2) The GBR related insns (e.g. store_gbr, *mov<mode>_gbr_load) use "register_operand" as destination. Since "register_operand" matches T_REG, a (set (reg:SI T_REG) (<gbr something>)) will be wrongly matched by any_treg_expr_to_reg. This should actually have ended in an unrecognized insn ICE, but then there's the *negtstsi insn, which results in funny code. The easy solution for this is to use "arith_reg_dest" instead of "register_operand" in the GBR insns. I'll send around an updated treg_set_expr patch after 1) is done. Cheers, Oleg
