This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] MIPS: Fix GCC `noreorder' for undefined R5900 short loops


Fredrik,

> >  Why?  What is wrong with using `-Werror'?
> > 
> >  Or you could use `-Wa,-fatal-warnings' to limit making warnings fatal to 
> > the assembly stage only if you are concerned about bad high-level language 
> > code quality interfering with your goal.
> 
> It appears unfeasible to fix thousands of build scripts to work like that.
> Although the numbers with actual MIPS code in them likely are very small,
> some may source macros and other stuff via various libraries. Some
> distributions, such as Gentoo, do have global CFLAGS but such mechanism are
> unreliable.

 I'd expect a sane distribution packager to prepare/patch any disobedient 
software packages to accept global distribution-specific flags, as there 
are often more important flags to pass which all packages need to be build 
with.

 However if they haven't, then put the flags in CC/CXX/etc. as you'd do 
with multilib selection options, or if that fails too, then wrap your 
compiler driver used for a distribution build into a shell script found 
according to $PATH (which may be non-standard) that adds any options 
required implicitly.  I've done that many times when I found myself in 
situation where I had to override some silly hardcoded assumptions, 
including removing or transforming rather than adding options passed.

> I understand that one may want to take a principled stance with "hands away"
> and "author asked for trouble", but fact is that whomever put a given
> noreorder directive into a piece of code most likely didn't have R5900
> errata in mind, and R5900 users most likely don't want the assembler to
> generate code that is known to cause subtle bugs of all imaginable kinds,
> including data corruption.

 Yes, and a warning is I think a reasonable solution.  Alternatively maybe 
you could persuade binutils maintainers to have a `configure' option to 
make `-fatal-warnings' GAS's default for a given build.

> > >  3:
> > > +	short_loop_war(3b)
> > >  	b	3b
> > >  	 nop
> > 
> >  Is it needed here given that the delay slot instruction is a NOP anyway?  
> 
> Ah, no. That is a left-over from the note that "a branch delay slot of the
> loop is not NOP (EE 2.9 or later)", as it appears a NOP is not enough for
> EE before 2.9.

 Ah, OK then.

> >  Also this source does not need `.set noreorder', except for the loop 
> > around `1:' where a data dependency is present with the delay slot 
> > instruction.
> 
> That data dependency can surely be reworked too, so as to make the whole
> piece reorderable?

 The SW and ADDIU instructions can surely be swapped, e.g.:

	PTR_LA	a0, _edata - 4
	PTR_LA	a2, _end
1:	addiu	a0, a0, 4
	sw	zero, 0(a0)
	bne	a2, a0, 1b

You can make it all `reorder' then.  Note that this preserves the final 
out-of-range store, which I gather is deliberate for performance reasons.

  Maciej


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]