[Bug driver/38864] Incorrect interaction between --with-arch=native and -mARCH
--- Comment #4 from rdsandiford at googlemail dot com 2009-01-24 09:48
---
Subject: Re: Incorrect interaction between --with-arch=native and -mARCH
"nemet at gcc dot gnu dot org" writes:
> --- Comment #3 from nemet at gcc dot gnu dot org 2009-01-24 02:47 ---
> It was actually Richard's mips.exp rewrite that removed { target { fixed_point
> } } from these tests.
>
> Richard, was this intentional? It seems to me that since fixed-point is not
> dependent on a command-line flag (-mdsp is orthogonal) the effective-target
> check is correct here.
>
> Should I revert it like this:
Yeah. Sorry, I hadn't realised you could actually disable _cc1_ support
for fixed point. (I knew you could disable libgcc support, but that's
not a problem for these compile-only tests.)
Richard
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38864
[Bug target/28126] gcc moves an expensive instruction outside of a conditional
--- Comment #12 from rdsandiford at googlemail dot com 2008-09-08 19:48 --- Subject: Re: gcc moves an expensive instruction outside of a conditional "daney at gcc dot gnu dot org" <[EMAIL PROTECTED]> writes: > Can we close this now? > > I think it is fixed. Sorry, this is still on the back-burner. I think the underlying problem is still there, even if it doesn't trigger for the original testcases. Richard -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28126
[Bug rtl-optimization/56494] [4.8 Regression] ICE in simplify_truncation, at simplify-rtx.c:619
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56494 --- Comment #4 from rdsandiford at googlemail dot com 2013-03-04 21:06:10 UTC --- "jakub at gcc dot gnu.org" writes: > Created attachment 29578 > --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29578 > gcc48-pr56494.patch Thanks for the fix. Looks good to me FWIW. Truncations can't be to a narrower mode (already asserted at the top of the function) so in some ways I think it would be clearer to have an "if ... else" rather than two "if"s. Richard
[Bug rtl-optimization/54369] Delayed-branch pass in reorg.c removes too many instructions
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54369 --- Comment #7 from rdsandiford at googlemail dot com 2012-09-01 08:16:23 UTC --- "ebotcazou at gcc dot gnu.org" writes: >> Are you going to check this in on the mainline? > > Not without Richard's approval, so I've CCed him now. Looks good, thanks. Please go ahead. Just out of interest, you said: > The root cause of the problem is that MIPS runs its own version of the dbr > pass > and doesn't make sure that barriers are correctly placed for it, unlike other > targets like SPARC. But how does SPARC do it? sparc_reorg starts: /* The only erratum we handle for now is that of the AT697F processor. */ if (!sparc_fix_at697f) return; /* We need to have the (essentially) final form of the insn stream in order to properly detect the various hazards. Run delay slot scheduling. */ if (optimize > 0 && flag_delayed_branch) dbr_schedule (get_insns ()); without any explicit call to cleanup_barriers (which like you say usually runs after machine_reorg but before delay_slots).
[Bug rtl-optimization/54369] Delayed-branch pass in reorg.c removes too many instructions
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54369 --- Comment #9 from rdsandiford at googlemail dot com 2012-09-01 09:41:08 UTC --- "ebotcazou at gcc dot gnu.org" writes: >> Looks good, thanks. Please go ahead. > > Thanks. On which branch(es)? Hmm, looks like this goes all the way back to 4.5, so as any as you've got the stamina for. It should be a low-risk change. Richard
[Bug middle-end/54862] [4.8 Regression] error: comparison between signed and unsigned integer expressions in simplify-rtx.c
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54862 --- Comment #4 from rdsandiford at googlemail dot com 2012-10-10 07:41:10 UTC --- Sorry for the breakage and thanks to Jakub for the fix. I agree UINTVAL is the right way to go FWIW.
[Bug tree-optimization/46309] optimization a==3||a==1
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46309 --- Comment #13 from rdsandiford at googlemail dot com 2012-10-31 22:36:48 UTC --- "eidletni at mail dot ru" writes: > Cool, thank you! +1, thanks Jakub
[Bug bootstrap/53249] [4.8 Regression] Bootstrap failure
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53249 --- Comment #9 from rdsandiford at googlemail dot com 2012-05-06 15:56:09 UTC --- "hjl.tools at gmail dot com" writes: > get_address_mode in dwarf2out.c works for this testcase. Yeah, that's what I was testing FWIW. If x32 wants to do this, then pretty much every instance of: targetm.addr_space.address_mode (MEM_ADDR_SPACE (mem)) needs to use get_address_mode instead. So I'm trying to convert all of them. (There's a duplicate copy in var-tracking.c, so the patch consolidates them into a general rtl.h function.)
[Bug rtl-optimization/53176] [4.8 Regression] gcc.dg/lower-subreg-1.c FAILs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53176 --- Comment #19 from rdsandiford at googlemail dot com 2012-05-06 19:17:03 UTC --- "olegendo at gcc dot gnu.org" writes: > --- Comment #18 from Oleg Endo 2012-05-06 > 18:09:37 UTC --- > On SH an issue popped up because lower-subreg would not split multi-word regs > anymore. Could somebody please have a look at comment #2 and the proposed > patch in PR 53250? Unfortunately I messed up the choice of cost routines in the original patch. I just committed the fix for that. The SH rtx_costs routine should now see (set (reg) (reg)) and (set (reg) (const_int 0)) rtxes, so you should be able to set the costs there. See: http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00425.html for a similar MIPS patch.
[Bug target/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands
--- Comment #13 from rdsandiford at googlemail dot com 2010-05-03 09:53 --- Subject: Re: [4.5/4.6 regression] ICE in reload_cse_simplify_operands "mkuvyrkov at gcc dot gnu dot org" writes: > --- Comment #10 from mkuvyrkov at gcc dot gnu dot org 2010-04-23 10:20 > --- > The problem seems to be in Richard's patch > (http://article.gmane.org/gmane.comp.gcc.patches/130602) checked in r120961. > > All and all, it seems that revision 120961 should be reverted to enable 'T' > constraint for (flag_pic && !TARGET_PCREL). > > The 's' constraint is defined as > == > case 's': > if (CONST_INT_P (op) > || (GET_CODE (op) == CONST_DOUBLE > && GET_MODE (op) == VOIDmode)) > break; > case 'i': > if (CONSTANT_P (op)) > win = 1; > break; > == > > So, unless I'm missing something, the statement > "... 's' doesn't accept anything if flag_pic" > is just wrong. I meant "flag_pic && !TARGET_PCREL", since only the !TARGET_PCREL case matters for 'T'. And that was right at the time that I wrote it. The interesting definition of 's' isn't the one you quote, but the one in reload: case 's': if (CONST_INT_P (operand) || (GET_CODE (operand) == CONST_DOUBLE && GET_MODE (operand) == VOIDmode)) break; case 'i': if (CONSTANT_P (operand) && (! flag_pic || LEGITIMATE_PIC_OPERAND_P (operand))) win = 1; break; That is, 's' operands have to satisfy LEGITIMATE_PIC_OPERAND_P when generating PIC. I'm not sure which version you were quoting, but if it was constrain_operands, that's a special case. constrain_operands can rely on the predicates to check for legitimate PIC operands, so there's no need to repeat the check there. At the time I committed the patch, LEGITIMATE_PIC_OPERAND_P was defined as follows: #define LEGITIMATE_PIC_OPERAND_P(X) \ (!symbolic_operand (X, VOIDmode) \ || (TARGET_PCREL && REG_STRICT_P)) Thus no symbolic constant was a legitimate PIC operand for flag_pic && !TARGET_PCREL. Thus nothing satisfied 's' when flag_pic && !TARGET_PCREL. The 'T' constraint is defined as follows: satisifies(T) == satisifies(s) && !TARGET_PCREL so it followed that nothing should match 'T' when flag_pic. So the patch was correct at the time it was committed. Please understand that reverting it is the wrong thing to do. It would reintroduce the original bug: that constraints _must not_ match something that the associated predicates do not. Only the other way is allowed: predicates can allow things that the constraints don't, within certain limits. For example, let's say you have an insn that matches: (define_insn "subsi3" [(set (match_operand:SI 0 "nonimmediate_operand" "=mda,m,d,a") (minus:SI (match_operand:SI 1 "general_operand" "0,0,0,0") (match_operand:SI 2 "general_src_operand" "I,dT,mSrT,mSrs")))] "" "@ subq%.l %2, %0 sub%.l %2,%0 sub%.l %2,%0 sub%.l %2,%0" [(set_attr "type" "aluq_l,alu_l,alu_l,alu_l") (set_attr "opy" "2")]) And let's suppose that operand 2 is a register that is equal to: (symbol_ref "x") If 'T' allows any CONST, SYMBOL_REF or LABEL_REF when flag_pic && !TARGET_PCREL (as in your suggested patch), reload could quite happily establish that operand 2 is (symbol_ref "x"), see that it matches 'T', and use it. This will then lead to an unrecognisable insn, because although the constant matches the 'T' constraint, it doesn't match general_src_operand. Instead, the bug is that the 'T' constraint wasn't updated by the TLS support at the same time as LEGITIMATE_PIC_OPERAND_P was. An easy thing to miss, of course. I think the correct patch is the one I'm about to attach. Richard -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43804
