Re: [RFC, Fortran] Avoid race on testsuite temporary files
Hi Janne, On 11 December 2015 at 12:12, Janne Blomqvist wrote: > On Wed, Dec 9, 2015 at 4:27 PM, Yvan Roux wrote: >> Hi, >> >> as it was raised in >> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg01540.html we experiment >> random failures in gfortran validation when it is run in parallel (-j >> 8). The issues occurs because of concurrent access to the same file, >> the first two patches fixed some of them by changing the file names >> used, but there are still remaining conflicts (6 usages of foo, 8 of >> test.dat). This is easy to fix and I've a patch for that, but there is >> another issue and I'd like to have your thoughts on it. >> >> There is a little bit more than 1000 testcases which use IO without >> explicit file names, ~150 use scratches (a temporary file named >> gfortrantmp + 6 extra char is created) and the others, which only >> specify the io unit number, use a file named fort.NN with NN the >> number of the io unit. We see conflicts on these generated files, as >> lot of testcases use the same number, the most used are: >> >> 10 => 150 >> 99 => 70 >> 6 => 31 >> 11 => 27 >> 1 => 23 >> >> I started to change the testcases to use scratches when it is possible >> and before finding that there is that many to fix, and I also had >> conflicts on the generated scratch names. The options I see to fix >> that are: >> >> 1- Move all these testcases into an IO subdir and change the testsuite >> to avoid parallelism in that directory. >> 2- Use scratches when possible and improve libgfortran file name >> generation, I don't know well fortran but is it possible to change the >> file name patterns for scratches and io unit files ? > > For the "io unit files", I don't think we can change it. fort.NN is a > common convention, and I suspect a lot of people rely on it. As for > scratch files, I'm surprised you're seeing conflicts. At least on > Linux/POSIX targets it should use mk(o)stmp(), which should make sure > a unique file name is chosen and opened with exclusive access > (O_CREAT|O_EXCL). Furthermore, on POSIX targets it unlinks the file > immediately after opening it, further reducing the possibility of any > conflict. So what target are you compiling for? OK if people rely on the fort.NN name pattern I understand it can't be changed. We're validating on x86, ARM and AArch64 linux targets and we experienced random issues with backslash_1.f90, given the nature of the testcase and the other issues with fort.NN conflict I assumed it was the same kind, but it's most likely due to some instabilities we had in our lab several months ago, as we have no more issues with it now, and my manual experiment of mk(o)stmp() usage didn't manage to create conflicts. So, Thanks for the clarification and sorry for the false alarm on scratches. >> 3- Change the io unit numbers used, as it was suggested on irc, but I >> find it a bit painful to maintain. > > Is it really that much more work than 1)? If you don't want to keep > track of a count, just generate a random number between 10 and 2^31, > and use that as the unit number? Should at least make conflicts rare. I don't think it is more work to fix the actual testsuite, my point of view is that 1) will guarantee that we will not have any conflicts no matter which IO units or temporary file names are or will be used, whereas if we keep them as it, with changing the numbers, maintainers and contributors will have to take care of choosing an IO unit or a temporary file name that is not already used in the testsuite. Thanks, Yvan
AW: basic asm and memory clobbers - Proposed solution
Hi,
On 15. Dezember 2015 23:43, Joseph Myers wrote:
> I think the typical use of basic asm is: you want to manipulate I/O
> registers or other such state unknown to the compiler (not any registers
> the compiler might use itself), and you want to do it in a way that is
> maximally compatible with as many compilers as possible (hence limiting
> yourself to the syntax subset that's in the C++ standard, for example).
> Compatibility with a wide range of other compilers is the critical thing
> here; this is not a GCC-invented feature, and considerations for
> deprecating an externally defined feature are completely different from
> considerations for GCC-invented features. Do you have evidence that it is
> now unusual for compilers to support basic asm without supporting
> GCC-compatible extended asm, or that other compiler providers generally
> consider basic asm deprecated?
I totally agree, and I know of another compiler which supports basic asm,
but no extended asm:
The windriver diab compiler does apparently not support extended asm,
instead they offer either "asm string statements" or "asm macros".
This is quoted from http://www.vxdev.com/docs/vx55man/diab5.0ppc/c-embed_.htm
"asm string statements are primarily useful for manipulating data in static
variables and special registers, changing processor status, etc., and are
subject to several restrictions: no assumption can be made about register
usage, non-scratch registers must be preserved, values may not be returned,
some optimizations are disabled, and more. asm macro functions described above
are recommended instead. See Notes and restrictions below.
An asm string statement provides a simple way to embed instructions in the
assembly code generated by the compiler. Its syntax is
asm[ volatile] ("string"[ ! register-list]);
where string is an ordinary string constant following the usual rules (adjacent
strings are pasted together, a `\' at the end of the line is removed, and the
next line is concatenated) and register-list is a list of scratch registers
(see Register-list line, p.154). The optional volatile keyword prevents
instructions from being moved before or after the string statement.
An asm string statement can be used wherever a statement or an external
declaration is allowed. string will be output as a line in the assembly code at
the point in a function at which the statement is encountered, and so must be a
valid assembly language statement.
If several assembly language statements are to be generated, they may either be
written as successive asm string statements, or by using `\n' within the string
to end each embedded assembly language statement. The compiler will not insert
any code between successive asm string statements.
If an asm string statement contains a label, and the function containing the
asm string is inlined more than once in some other function, a duplicate label
error will occur. Use an asm macro with a storage mode line containing a lab
clause for this case. See asm macros, p.151. "
asm macros on the other hand are very sophisticated, but totally incompatible
with gcc
Also from there:
Examples of asm macros
In this example, standard PowerPC techniques are used to wait on and then seize
a semaphore when it becomes free.
asm void semaphore_seize (volatile int *semaphore_p)
{
% reg semaphore_p; lab loop
! "r4", "r5"/* scratch registers used */
addir4,r0,1 # token for semaphore
loop: # label replaced by compiler
lwarx r5,r0,semaphore_p # semaphore will be in register
cmpicr0,0,r5,0 # semaphore free?
bne loop# branch if no
stwcx. r4,r0,semaphore_p # store token in semaphore
bne loop# in case interrupted before stwcz.
}
So, yes, you are right, the only globally understood form of assembler
statements in C is basic asm.
Bernd.
gcc-4.9-20151216 is now available
Snapshot gcc-4.9-20151216 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/4.9-20151216/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 4.9 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-4_9-branch revision 231723 You'll find: gcc-4.9-20151216.tar.bz2 Complete GCC MD5=c6b31e39048f1873efbe24278619db38 SHA1=eee840bb5160527cc97879bd3c69011b63cd34ed Diffs from 4.9-20151209 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-4.9 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.
Re: basic asm and memory clobbers - Proposed solution
On 12/15/2015 12:42 PM, paul_kon...@dell.com wrote:
In the codebase for the product I work on, I see about 200 of them. Many of those are the likes of
asm("sync") for MIPS, which definitely wants to be treated as if it were asm ("sync" : :
: "memory").
That's right, I meant to ask you about this last time you mentioned this.
Now that you are aware that this is a problem, what do you intend to do
about it? Jeff is saying that this may not be fixed until at least v7,
so waiting for a compiler fix may take a while.
Will you be updating your source?
Are you just finding these with grep, or have you tried the
-Wonly-top-basic-asm patch?
That's not counting the hundreds I see in gdb/stubs -- those are "outside a
function" flavor.
Fortunately, these aren't a problem for memory or register clobbers.
dw
Re: basic asm and memory clobbers - Proposed solution
On 12/15/2015 1:13 PM, Jeff Law wrote: Sadly, I'm putting most of this discussion into my gcc-7 queue anyway. Fair enough. If "clobbers" is what we're going to do, that sounds more like a phase 1 thing. That said, some people who have this problem may prefer to fix it sooner rather than later. If we soften the text for the docs to remove the word "deprecate," would just adding -Wonly-top-basic-asm (non-default) be useful for v6? dw
Re: basic asm and memory clobbers - Proposed solution
On 12/15/2015 5:01 PM, paul_kon...@dell.com wrote: On Dec 15, 2015, at 5:22 PM, David Wohlferd wrote: On 12/14/2015 1:53 AM, Andrew Haley wrote: This just seems like another argument for deprecating basic asm and pushing people to extended. Yes. I am not arguing against deprecation. We should do that. You know, there are several people who seem to generally support this direction. Not enough to call it a consensus, but perhaps the beginning of one: - Andrew Haley - David Wohlferd - Richard Henderson - Segher Boessenkool - Bernd Schmidt Anyone else want to add their name here? No, but I want to speak in opposition. Fair enough. "Deprecate" means two things: warn now, remove later. Yup. That's what I'm proposing. Although "later" could be a decade down the road. That's how long 24414 has been sitting. For reasons stated by others, I object to "remove later". So "warn now, remove never" I would support, but not "deprecate". So how about: - Update the basic asm docs to describe basic asm's current (and historical) semantics (ie clobber nothing). - Emphasize how that might be different from users' expectations or the behavior of other compilers. - Warn that this could change in future versions of gcc. To avoid impacts from this change, use extended. - Mention -Wonly-top-basic-asm as a way to locate affected statements. Would that be something you could support? What's your take on making -Wonly-top-basic-asm a default (either now or v7)? Is making it a non-default a waste of time because no one will ever see it? Or is making it a default too aggressive? What about adding it to -Wall? dw
Re: basic asm and memory clobbers - Proposed solution
On 12/15/2015 2:43 PM, Joseph Myers wrote: On Tue, 15 Dec 2015, David Wohlferd wrote: Unlike top level, using basic asm within a function is deprecated. No new code should use this feature, but should use extended asm instead. Existing code should begin replacing such usage. Instances of affected code can be found using -Wonly-top-basic-asm. For help making this conversion, see "How to convert Basic asm to Extended asm." I think the typical use of basic asm is: you want to manipulate I/O registers or other such state unknown to the compiler (not any registers the compiler might use itself), and you want to do it in a way that is maximally compatible with as many compilers as possible (hence limiting yourself to the syntax subset that's in the C++ standard, for example). Compatibility with a wide range of other compilers is the critical thing here; this is not a GCC-invented feature, and considerations for deprecating an externally defined feature are completely different from considerations for GCC-invented features. Do you have evidence that it is now unusual for compilers to support basic asm without supporting GCC-compatible extended asm, or that other compiler providers generally consider basic asm deprecated? On the contrary, I would be surprised to learn that there are ANY compilers (other than clang) that support gcc's extended asm format. And although there is no standard that seems to require it, I'm not certainly not prepared to say that basic asm is "generally deprecated." But the fact that there is no standard may make "doing what other compilers do" challenging. For example quoting from Bernd's email regarding the windriver diab compiler: "non-scratch registers must be preserved." Implying that scratch registers (which they apparently only list for ARM) are considered clobbered. That seems like a sensible approach (and avoids the frame pointer problem). But I'm not prepared to extrapolate from that how all compilers do or should handle basic asm. However it does mean that the suggestion being proposed here to have basic asm only clobber memory would not be compatible with windriver's approach to basic asm. And Jeff's proposal for gcc to clobber "all registers" wouldn't be compatible with them either. I agree that "oh, surprise! gcc does this differently than compiler X!" is a bad thing. But without standards, trying to be compatible with how "everyone else" does it may not be practical. You'll note that windriver made no particular effort to be compatible with gcc. And of course any change will make gcc v7 work differently than gcc v4, v5, v6. If compatibility with other compilers is an important criteria when determining how gcc should handle basic asm, someone's going to need to do some research and some prioritizing. In the meantime, raising awareness of this issue via docs and warnings seems a low-cost way to start. dw
[RFC] ICE when error_mark_node is gimplified
Hi, PR 68948 shows a case which has been broken for a long time and very hard to see. So I am recommending that we introduce an assert inside the gimplifier if we process an error_mark_node and there was no errors or sorrys from the front-end; I will send a patch if there is a general feeling that it is a good idea. Good idea or bad idea? This will prevent cases like PR 68948 from happening and make it easier to understand why the wrong code is happening. Thanks, Andrew
