This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
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