RE: Solve transitive closure issue in modulo scheduling
"Bingfeng Mei" wrote on 05/02/2009 12:45:20: > Ayal, > OOPs, your mail skipped my inbox and I missed it for several days. > I'm chasing after my mail as well.. > Using testsuite/gcc.dg/sms-6.c as an example and compiling it for PowerPC, > node 18 (see attachment) is in a SCC and cannot be scheduled until spliting > twice. The MII = 20 and the schedule can only be found at II = 24. Yes, I see. This example raises a couple of issues: o The first row split (from II=20 to II=21) is miscalculated; it should be row 20=0 instead of 19. Splitting row 19 cannot help schedule node 18, and indeed we immediately split another row. We're now checking a small patch to fix this, which should save one cycle of II in the above example. o The swinging scheduling procedure tends to create sets of densly scheduled instructions with holes between them. For example, the set of nodes 3, 7, 11, 15 first scheduled adjacently before (without leaving room for) node 18 (in cycles 6 (=26 mod 20), 25, 24, 23 respectively). So it may be possible to delete some empty row from such a hole (suffices to check a single row from each hole), after we insert an empty row to accommodate an instruction, thereby maintaining the II. This should save at-least one additional cycle of II in the above example. o Transitive closure can indeed prevent the construction of infeasible partial schedules. Note that you'd be better off with a dense incidence representation for the DDG, as it will become a complete graph. The transitive DDG could be pruned according to the scheduling order, as we're only interested in adding edge u -> v if there is some w on a path u ~> w ~> v where w follows both u and v in the to-be-scheduled order; but for that you probably need to have transitive edges u -> w and w -> v ... Plus, leaving a single cycle for w to be scheduled between u and v may not be enough, as other instructions or conflicts may prevent w from eventually being scheduled in this cycle. So the split (&remove) row mechanism may still be useful. o Another preventive (rather than corrective) action can be to prioritorize register dependent nodes over memory dependent ones. Note that in the example above, the 4 predecessors (7,11,15,18) of node 3 are equally critical, and we're missing some tie-breaking criteria. It may be better to prioritize node 18 due to its register dependence (the general motivation being that near-by nodes imply shorter register live-ranges) over the other nodes of output memory dependence. It's not easy however to extend the current infrastructure to consider such criteria. > On our 2- > issue VLIW, the MII=10 and the valid schedule can only be found at II = 14. It > is not great since we want to maximize performance. > > I had experience (in development of another compiler) on this issue by > constructing the MinDist matrix and using it to calculate schedule window for > each instruction. However, speed was not a real concern then. Do you know > better algorithm (better than O(N^3))? No, I don't. As mentioned above, we might be able to save something by not calculating the complete closure. > Or do you think it is not so crtical > here? After all, not many loops are candidates for software pipelining. Thanks. > I would suggest to check the actual compile-time and memory usage, and try to reduce them later if needed. Modulo-scheduling is not a compile-time cheap optimization in general. Ayal. > Cheers, > Bingfeng > > > -Original Message- > > From: gcc-ow...@gcc.gnu.org [mailto:gcc-ow...@gcc.gnu.org] On > > Behalf Of Ayal Zaks > > Sent: 01 February 2009 23:18 > > To: Bingfeng Mei > > Cc: Adrian Ashley; gcc@gcc.gnu.org > > Subject: Re: Solve transitive closure issue in modulo scheduling > > > > "Bingfeng Mei" wrote on 30/01/2009 14:44:01: > > > > > Hello, > > > I try to make modulo scheduling work more efficiently for our VLIW > > target. I > > > found one serious issue that prevents current SMS algorithm from > > achieving > > > high IPC is so-called "transitive closure" problem, where scheduling > > window is > > > only calculated using direct predecessors and successors. > > Because SMS is > > not > > > an iterative algorithm, this may cause failures in finding a valid > > schedule. > > > > Agreed. > > > > > Without splitting rows, some simple loops just cannot be > > scheduled not > > matter > > > how big the II is. With splitting rows, schedule can be > > found, but only > > at > > > bigger II. > > > > It may happen that even splitting rows will not help, e.g. when we > > repeatedly end up with a node having negative sched window. > > > > > GCC wiki (http://gcc.gnu.org/wiki/SwingModuloScheduling) lists this > > > as a TODO. Is there any work going on about this issue > > > > No, not to my knowledge. We had some testcase where this > > showed up, hence > > its appearance in the TODO, but afaicr some change caused it > > to disappear. > > > > > (the last wiki update > > > was one year ago)? If no one is workin
Re: changed_allocation_pseudos
Jeff Law wrote: What purpose does changed_allocation_pseudos serve? AFAICT we set/clear the bitmap, but never use it for anything. It was added as part of the IRA integration. Did you have some purpose in mind for this bitmap? If not can we just remove it? As I remember, it was used in an optimization which was never merged into the trunk because of its complexity and tiny impact on performance. The optimization was to reassigning caller-used hard registers freed by caller-save.c to spilled pseudos by IRA or reload. Apparently my removing the optimization from IRA patch not was not clean and the bitmap has survived. I think we should remove it because it is not used at all. I'll submit a patch for this. Thanks for finding this, Jeff.
Re: IRA conflict graph & alternative selection
Jeff Law wrote: Ian Lance Taylor wrote: Jeff Law writes: Ian Lance Taylor wrote: I see no reason for those to stay in reload (especially since I think reload should disappear entirely). It is reasonable to pick the total maximum size of the stack frame, and thus resolve all displacement constraints, before register allocation. Carefully relaxing these constraints during reload can give you slightly better results for some instructions, but only in very very few cases, and only in functions which already have unusually large stack frames. I don't consider that to be an important optimization. Given that, we can determine the maximum offset for all virtual registers before register allocation, which suffices for selection of insn constraint alternatives, and then determine the actual offset, once, after register allocation. I would agree that careful relaxation of displacements is no longer as important as it once was, I don't think we can just hand wave away the displacement issues 1. The stack frames don't have to be that big to bump up against these problems. 2. The code we generate if we have to reload the address because the displacement was out of range can be horrific 3. There are targets where other registers used in the insn determine the range of the displacement. ie, in a load from memory, the destination register used determines the valid range of displacements (+-16 bytes vs +-8k on one target I'm aware of. In all of thse cases, the relaxation loop can only affect a handful of instructions: the cases where saving a few less registers moves the offset within range. Those few instructions can only occur in a handful of functions: the ones where the stack frame is so large that this becomes an issue at all. I disagree, particularly because of point #3.I don't see how you can hand wave it away, that is unless you plan on just making every load/store of a stack variable/spill be assumed to be out of the +-16 byte range which will generate absolutely horrible code. On that particular target is isn't uncommon to have situations where you think you're going to be able to use the +-8k instruction, but because of spilling you end up using a different register and suddenly you're stuck with only being able to use +-16 byte offsets. According my old notes, there are plenty targets with small displacement issues: avr (0:64), c4x (-255:255), fr30 (-512:512 or 0:64 depending on data mode), ip2k (0:127), m68c11(0:256), m68k(-128:127), mcore(0:15, 0:30 etc depending on date mode), mmix (0:255), xtensa (0:255 for QI) and famous sh (0:63). I think not relaxing the displacements will really hurt the performance especially for targets with small register file like mcore. IMHO, another important optimization what reload does for such processors is usage of already calculated displacement whose value is in hard register to calculate an new displacement (that was implemented by Joern for SH). Although it is hard for me to say how these optimizations will complicate the new implementation. May be we could sacrifice a small performance degradation for more clear code. I don't know, only two tried implementations will show it.
Re: IRA conflict graph & alternative selection
Jeff Law wrote: Vladimir Makarov wrote: Jeff Law wrote: I've been thinking further about instruction alternative selection prior to allocation and one of the questions in my mind is how this interacts with IRA. We select an alternative for each insn based on some "best guess" heuristic -- the selection of an alternative will often restrict the register classes available for each of the operands. Presumably we'd want to encode that information it the conflict graph so that IRA would allocate registers so as to fit the constraints of the early insn alternative selection. Right? In the case where the graph is uncolorable, do we allow IRA to override the alternative selection, or do we insert copies to simplify the conflict graph or some mixture of both? Thoughts? As for copies, I think it would be a bad decision to stick only to original (after the code selection) alternative and generate copies to satisfy this alternative. For example, if pseudo got memory instead of hard-register required by the alternative, it would be bad to generate a copy (ld/st in this case) if memory is accepted by the insn. That's why I mentioned the possibility of relaxing the conflict graph to allow other alternatives if we find that the graph is uncolorable.So if we initially wanted class A, but couldn't get it and the operand could accept class B, then we remove the conflict between the pseudo and the hard regs in class B and recolor. I have no idea how expensive this would be. This also implies that we're representing conflicts for register classes & memory in the conflict graph. The problem is that graph coloring working well on non-intersected register classes. There were some articles how to make it work on intersected register classes. The most known is "A Generalized Algorithm for Graph-Coloring Register Allocation": http://www.cs.tufts.edu/~nr/pubs/gcra-abstract.html I read this article with attention several times and even wrote some notes but unfortunately can not find it. I got an impression that it will not work. As for changing register class from one cover class (e.g. GENERAL_REGS) to another non-intersected one (FLOAT_REGS), it is possible but more rare than changing register class in one cover class (like from GENERAL to AREG in x86). The most common example is usage pseudo for moving memory values (e.g. generated from a[i] = b[i]). It can be done through int or fp regs. I've tried some graph coloring algorithm modification to permit register class changing when register pressure for original class is high and one for new class is low but got mixed results on x86 and ppc (some tests were worse some were better with practically the same average result).
Re: IRA conflict graph & alternative selection
Ian Lance Taylor wrote: > No, that makes no sense. What I'm suggesting is that we fix the stack > offsets of all local variables before register allocation, based on a > conservative assessment of how many registers will be saved on the > stack. The conservative assessment is that all pseudos go on the stack. However, this way you'll generate terrible code. I don't really understand why people want to remove reload only to implement the same thing elsewhere. There are only two major problems with reload IMO: - the RELOAD_FOR_xxx mechanism of scheduling reload insns is terrible - so is the inheritance code Even so, the number of bugs in these seems to have dropped off over the years. If you replace these two with a cleaner solution, you'll end up with a fairly clean and easy to understand reload pass. We'd still want the register allocator to be strong enough not to leave too much work for it, but I simply don't see how reload can be entirely replaced in gcc. Bernd -- This footer brought to you by insane German lawmakers. Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368 Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif
Re: IRA conflict graph & alternative selection
Ian Lance Taylor wrote: Jeff Law writes: I would agree that careful relaxation of displacements is no longer as important as it once was, I don't think we can just hand wave away the displacement issues 1. The stack frames don't have to be that big to bump up against these problems. 2. The code we generate if we have to reload the address because the displacement was out of range can be horrific 3. There are targets where other registers used in the insn determine the range of the displacement. ie, in a load from memory, the destination register used determines the valid range of displacements (+-16 bytes vs +-8k on one target I'm aware of. In all of thse cases, the relaxation loop can only affect a handful of instructions: the cases where saving a few less registers moves the offset within range. Those few instructions can only occur in a handful of functions: the ones where the stack frame is so large that this becomes an issue at all. I disagree, particularly because of point #3.I don't see how you can hand wave it away, that is unless you plan on just making every load/store of a stack variable/spill be assumed to be out of the +-16 byte range which will generate absolutely horrible code. No, that makes no sense. What I'm suggesting is that we fix the stack offsets of all local variables before register allocation, based on a conservative assessment of how many registers will be saved on the stack. Then we know during register allocation whether the memory reference will be in or out of the +- 16 byte range. Just fixing the offset is not sufficient for the case I'm concerned about because you don't know what offsets are valid until you know precisely what registers are used in the insn. That's absolutely critical. So you have to assume worst case which is you can only use the +-16 byte insns which will pessimize just about every stack reference. Perhaps the confusion in this case is because we're not really relaxing -- we're dealing with a case where the offset alone isn't enough to determine if the addressing mode is valid. What we lose is the ability to discover that our conservative assessment was overly conservative, and so actually some small number of instructions will be generated as out of range when they could have been in range. (Of course we will pick up some of those cases using peepholes). In the case I'm talking about, it will affect a huge number of insns. Without knowing the size of the frame, how do you plan on doing this without making the assumption that nothing is going to fit in the shorter displacement variants? How can you do this when the range of valid displacements can change because the register you used got spilled and you got a register from a different class (which in turn has a drastically smaller set of valid displacements). I'm saying that you guess the size of the frame, so your premise does not describe the aproach that I am suggesting. But the offsets are relative to the size of the frame. So I don't see how you can set the offsets without knowing the size of the frame. Clearly neither of us is understanding what the other is saying. Jeff
Re: IRA conflict graph & alternative selection
Vladimir Makarov wrote: IMHO, another important optimization what reload does for such processors is usage of already calculated displacement whose value is in hard register to calculate an new displacement (that was implemented by Joern for SH). ISTM this optimization could be done with a pre-variant rather than the ad-hoc implementation we have now. Jeff
Does GCC C support nested function definitions
Is it intended that the GCC C compiler support nested functions? I would like to know so that I can determine whether to file bug reports against other projects for nested function use. Begin Nested Function example void foo(void) { void bar(void) { puts("Bar"); } puts("Foo"); bar(); } End Nested Function example The last I heard, the ANSI (ISO) C language specification prohibited nested functions. I have been looking at the code for GRUB, another GNU project, and noticed that there are several instances of nested functions. I asked one of the maintainers of GRUB and received the following response which implies that: 1) the GCC C compiler is intended to support nested functions 2) Grub (and possibly other GNU projects) are written in "GNU C" and not intended to be compiled with other compilers. Begin Quote > 2) grub/asmstub.c :: grub_stage2() > This function knowingly uses a nested function. Nested > functions are prohibited by the ISO-C and C++ language > specifications. Conformant compilers will blow up. It seems that grub is written in GNU C, not ISO-C. So it is not intended to be compiled by other compilers. There is many nested functions used in grub source code. So, don't worry about that. End Quote Thank You, Daryl McDaniel SSG/SSD/Platform Software Infrastructure JF1-235 +1 503-712-4670 Simple clear purpose and principles give rise to complex and intelligent behavior. Complex rules and regulations give rise to simple and stupid behavior. -- Dee Hock
Re: Does GCC C support nested function definitions
> Is it intended that the GCC C compiler support nested functions? Yes, it's an extension provided by GNU C, not part of ISO C. Arno
Re: Does GCC C support nested function definitions
On Tue, Feb 17, 2009 at 9:55 AM, Arnaud Charlet wrote: >> Is it intended that the GCC C compiler support nested functions? > > Yes, it's an extension provided by GNU C, not part of ISO C. > Although nested functions should be avoided for portability, but sometimes nested functions are very useful for low level codes. Glibc is another project which depends on nested functions. -- H.J.
Re: GCC and the Visual Basic programmer....
However, a proportion of code written for Visual C++ makes use of propriatery runtimes such as MFC, the runtime EULA of which 'currently' prevents the use of MFC based applications with a 'free' OS like ReactOS or GNU based toolchains... And even if it were permitted, it would be a bad thing to use it, since it is non-free (user-subjugating) software. Should there be an alternate but compatible implementation of MFC? It would be very useful to implement compatible interfaces but make it run on GNU and Unix systems. Should there be a way of using Visual Basic style code without using the vendors runtime? Yes. It would be good to add to GCC a front end for Visual Basic. Technically speaking, I've been told VB used a p-code form rather than direct compliation to native code, so .. Is there a way to automate the conversion/loading of this p-code into form that would compile with with a GNU derived toolchain? Since that would still mean using non-free software, it isn't a solution. It's just a different form of the same ethical problem. A solution means doing it without proprietary software.
Re: Does GCC C support nested function definitions
H.J. Lu wrote: On Tue, Feb 17, 2009 at 9:55 AM, Arnaud Charlet wrote: Is it intended that the GCC C compiler support nested functions? Yes, it's an extension provided by GNU C, not part of ISO C. Although nested functions should be avoided for portability, but sometimes nested functions are very useful for low level codes. Glibc is another project which depends on nested functions. And BTW, the Ada front end *heavily* relies on the back end capability of supporting nested functions (I assume the same is true of the Pascal front end). Note that nested functions are particularly useful where you want the effect of global variables, without the thread-unsafety that comes from real globals.
Re: IRA conflict graph & alternative selection
On Tue, Feb 17, 2009 at 5:51 PM, Bernd Schmidt wrote: > The conservative assessment is that all pseudos go on the stack. > However, this way you'll generate terrible code. Yes. So make a not-too-optimistic estimate. And if it turns out your estimate is too small, then you roll back, update the estimate, and try again. This only works if you can make the "roll back" part work, but it's been done before (not for GCC, of course). > I don't really understand why people want to remove reload only to > implement the same thing elsewhere. I don't think "people" want to remove reload. Simplifying it, is probably a better description. Splitting out tasks not related to spilling. Trying to do the same things in a place where you can still create a pseudo, instead of trying really hard to find a register and change many choices of the register allocator while at it. > There are only two major problems > with reload IMO: > - the RELOAD_FOR_xxx mechanism of scheduling reload insns is terrible > - so is the inheritance code > Even so, the number of bugs in these seems to have dropped off over the > years. There is at least one more major problem with reload: It performs instruction selection. A big issue with register allocation in GCC is that the register allocator doesn't even really know what instruction it is allocating registers for! Gr. Steven
Re: IRA conflict graph & alternative selection
Jeff Law writes: >> No, that makes no sense. What I'm suggesting is that we fix the stack >> offsets of all local variables before register allocation, based on a >> conservative assessment of how many registers will be saved on the >> stack. Then we know during register allocation whether the memory >> reference will be in or out of the +- 16 byte range. > Just fixing the offset is not sufficient for the case I'm concerned > about because you don't know what offsets are valid until you know > precisely what registers are used in the insn. That's absolutely > critical. So you have to assume worst case which is you can only use > the +-16 byte insns which will pessimize just about every stack > reference. You fix the offset of the value stored on the stack. Given that, you know which instruction you can use. > Perhaps the confusion in this case is because we're not really > relaxing -- we're dealing with a case where the offset alone isn't > enough to determine if the addressing mode is valid. The confusion may be that we may be talking about different things. I'm talking about removing this loop in the reload function: /* This loop scans the entire function each go-round and repeats until one repetition spills no additional hard regs. */ >>> Without knowing the size of the frame, how do you plan on doing this >>> without making the assumption that nothing is going to fit in the >>> shorter displacement variants? How can you do this when the range of >>> valid displacements can change because the register you used got >>> spilled and you got a register from a different class (which in turn >>> has a drastically smaller set of valid displacements). >>> >> >> I'm saying that you guess the size of the frame, so your premise does >> not describe the aproach that I am suggesting. >> > But the offsets are relative to the size of the frame. So I don't see > how you can set the offsets without knowing the size of the frame. You know the size of the frame, by guessing conservatively as to how many registers will be required. Ian
Re: IRA conflict graph & alternative selection
Bernd Schmidt writes: > Ian Lance Taylor wrote: > >> No, that makes no sense. What I'm suggesting is that we fix the stack >> offsets of all local variables before register allocation, based on a >> conservative assessment of how many registers will be saved on the >> stack. > > The conservative assessment is that all pseudos go on the stack. > However, this way you'll generate terrible code. I think the number of pseudos on the stack is irrelevant. You put each one farther down the stack at a known offset. The only interesting number to choose is how many registers will need to be saved on the stack. > I don't really understand why people want to remove reload only to > implement the same thing elsewhere. There are only two major problems > with reload IMO: > - the RELOAD_FOR_xxx mechanism of scheduling reload insns is terrible > - so is the inheritance code > Even so, the number of bugs in these seems to have dropped off over the > years. The problem with reload is that it interferes with register allocation. Even if gcc had a perfect register allocator, we would still generate suboptimal code because reload would mess up the allocation. The key to solving that problem is that we need to do instruction selection (in gcc terms, picking which insn alternative to use) either before register allocation or in conjunction with it. Ian
Re: Incomplete Type on Pass By Value bug in g++ <4.3.0
On Mon, 2009-02-16 at 10:09 -0800, Janis Johnson wrote: > On Fri, 2009-02-13 at 12:28 -0800, Joe Buck wrote: > > On Fri, Feb 13, 2009 at 11:03:51AM -0800, Anthony Newnam wrote: > > > Thanks Joe. > > > > > > As far as I know the problem I'm seeing isn't a regression but perhaps > > > this script could still be useful. I don't really understand how it is > > > supposed to work, since it doesn't appear be working off svn updates. > > > > I haven't looked at it in years, so I can't help you there. When Janis > > first wrote the script gcc was still using CVS. But it should be useful > > as a starting point. > > I've been using a different version since we moved to Subversion, and > have intended to add it to contrib/. Maybe I'll do that now! > > > > Should I do something like a binary svn search between revisions > > > 124707 and 132947? It takes such a long amount of time to compile g++, > > > almost a half an hour with my quad core, that it didn't seem practical > > > try to do build so many times. I guess there is probably a way to > > > build g++ without the rest of gcc, but I haven't seen an option for > > > it. > > > > Yes. It would suffice to only build phase 1 of g++, without any libraries, > > to do what you want, so that should be faster. You don't need a full > > bootstrap. If the endpoints you list are correct, that's 8240 revisions > > to search, but a binary search only needs to try ceil(log2(8240)) > > revisions, or 14. So if each build and test takes 30 minutes, you'll > > have your answer in seven hours. > > I'll include my build script for the compiler only, which works back to > early 2003. > > Please ping me daily until I do this! I've checked in the newer regression hunt scripts, details in gcc-patches. Janis
[Ada] Wrong code in gcc/ada/a-teioed.adb causing FAIL of ACATS cxf3a01 on mipsel and ia64
Hi Robert, Since there's only one ACATS FAIL on mipsel-linux I investigated it and it looks like a-teioed.adb code is wrong in some case: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39221 << ,.,. CXF3A01 ACATS 2.5 09-02-17 22:07:04 CXF3A01 Check that the Valid function from package Ada.Text_IO.Editing returns False for strings that fail to comply with the composition constraints defined for picture strings. Check that the Valid function returns True for strings that conform to the composition constraints defined for picture strings. * CXF3A01 Incorrect result from Function Valid using Invalid_String = 9. CXF3A01 FAILED . http://gcc.gnu.org/ml/gcc-testresults/2009-02/msg01635.html After investigation it looks like the code of Expand is wrong when Picture'Length > MAX_PICSIZE since in this case memory after Result will be overwritten (on all platform, but with effect FAIL on mipsel and ia64-linux). Two obvious solutions: use Unsupress locally since there's already a others handler or add explicit length checks. >> To reach quickly the interesting point under gdb: break cxf3a01.adb:110 cond 1 i=25 run step If you share my analysis, what solution do you suggest? Thanks in advance, Laurent
Re: [Ada] Wrong code in gcc/ada/a-teioed.adb causing FAIL of ACATS cxf3a01 on mipsel and ia64
Laurent GUERBY wrote: Two obvious solutions: use Unsupress locally since there's already a others handler or add explicit length checks. analysis looks right, an explicit length check is more appropriate, better to avoid the exception. To reach quickly the interesting point under gdb: break cxf3a01.adb:110 cond 1 i=25 run step If you share my analysis, what solution do you suggest? Thanks in advance, Laurent
Re: [Ada] Wrong code in gcc/ada/a-teioed.adb causing FAIL of ACATS cxf3a01 on mipsel and ia64
On Tue, 2009-02-17 at 16:05 -0500, Robert Dewar wrote: > Laurent GUERBY wrote: > > > Two obvious solutions: use Unsupress locally since there's already a others > > handler or add explicit length checks. > > analysis looks right, an explicit length check is more appropriate, > better to avoid the exception. Is the following okay to commit if it passes testing? Thanks in advance, Laurent 2009-02-17 Laurent GUERBY * a-teioed.adb (Expand): Fix Result overflow. Index: a-teioed.adb === --- a-teioed.adb(revision 144242) +++ a-teioed.adb(working copy) @@ -84,6 +84,10 @@ -- character has already been made, so a count of one is a -- no-op, and a count of zero erases a character. + if Result_Index + Count - 2 > Result'Last then + raise Picture_Error; + end if; + for J in 2 .. Count loop Result (Result_Index + J - 2) := Picture (Picture_Index - 1); end loop; @@ -98,6 +102,10 @@ raise Picture_Error; when others => + if Result_Index > Result'Last then + raise Picture_Error; + end if; + Result (Result_Index) := Picture (Picture_Index); Picture_Index := Picture_Index + 1; Result_Index := Result_Index + 1;
Re: [Ada] Wrong code in gcc/ada/a-teioed.adb causing FAIL of ACATS cxf3a01 on mipsel and ia64
Laurent GUERBY wrote: On Tue, 2009-02-17 at 16:05 -0500, Robert Dewar wrote: Laurent GUERBY wrote: Two obvious solutions: use Unsupress locally since there's already a others handler or add explicit length checks. analysis looks right, an explicit length check is more appropriate, better to avoid the exception. Is the following okay to commit if it passes testing? Thanks in advance, Laurent 2009-02-17 Laurent GUERBY * a-teioed.adb (Expand): Fix Result overflow. Index: a-teioed.adb === --- a-teioed.adb(revision 144242) +++ a-teioed.adb(working copy) @@ -84,6 +84,10 @@ -- character has already been made, so a count of one is a -- no-op, and a count of zero erases a character. + if Result_Index + Count - 2 > Result'Last then + raise Picture_Error; + end if; + for J in 2 .. Count loop Result (Result_Index + J - 2) := Picture (Picture_Index - 1); end loop; @@ -98,6 +102,10 @@ raise Picture_Error; when others => + if Result_Index > Result'Last then + raise Picture_Error; + end if; + Result (Result_Index) := Picture (Picture_Index); Picture_Index := Picture_Index + 1; Result_Index := Result_Index + 1; looks ok 2 me
Re: [plugins] Comparison of plugin mechanisms
Hi Grigori and Zbigniew, I just took a look at the wiki page you guys put together. While I have some reservations about categorizing the plugin APIs into "production", "research", and "new pass", I totally agreed with you that the plugin support should be implemented in layers, starting from the core support that provides basic APIs and exposes all the internal GCC data structures, to more complicated support that allows more abstract APIs and maybe drastic changes in GCC compilation pipeline. I agree with Basile that the support for pass management should not be in a different category. I think allowing plugin modules to hook new passes into GCC is an important requirement for plugins to be really useful, so we should provide basic pass management support (such as adding a new pass or replacing an existing pass) even in the core (or "production") APIs. And in the advanced/extended (or "research") APIs, we can allow the whole pass manager to be replaced (probably like what you did in ICI). In our plugin prototype, we have implemented basic support for adding new passes and defined APIs that allow the plugin writers to specify where/when to hook in the new passes. I will modify the GCC Plugin API wiki with our proposal so that people can comment on it. You mentioned that you will be sending out a patch this week. Will your patch contains the implementation for both the "production" and "research" APIs? If your patch contains only the research APIs, we can prepare an official patch for the "production" APIs (based on the patch that I send out a while back ago) and send it out for review this week (if no one else would like to do it, that is). Thanks, Le-chun On Fri, Feb 13, 2009 at 1:26 AM, Grigori Fursin wrote: > Hi Basile et al, > > Thanks a lot for detailed explanations. > > In addition to Zbigniew's reply: > I didn't specifically put MELT into new category (I actually just > updated the page and added it to the other categories too), > but I still keep the new pass integration plugin, since > some of our colleagues would like to add new passes including > new optimizations, but have difficulties to do that since there > is no full documented API, they may not want to write it in C > (as you mentioned) and they are concerned about support overheads > if the internal API will be changing all the time. Providing > standard API would be of great help for such cases, but it rises > other issues such as opening GCC to proprietary plugins, etc > which has been heavily discussed here for some time. So I personally > think we should leave it for the future and just implement minimalistic > Plugin API that will already make many current and prospective users > happy and then we can gradually extend it ... > > By the way, Sean, what do you think about the proposed APIs? > Will it suit your purposes or some changes are also required? > I would like to add info about your plugin system but just > don't have much knowledge about that ... > > Cheers, > Grigori > >> -Original Message- >> From: Basile STARYNKEVITCH [mailto:bas...@starynkevitch.net] >> Sent: Friday, February 13, 2009 8:38 AM >> To: Grigori Fursin >> Cc: 'Diego Novillo'; gcc@gcc.gnu.org; 'Sean Callanan'; 'Taras Glek'; >> 'Le-Chun Wu'; 'Gerald >> Pfeifer'; 'Zbigniew Chamski'; 'Cupertino Miranda' >> Subject: Re: [plugins] Comparison of plugin mechanisms >> >> Hello All, >> >> >> Grigori Fursin wrote: >> > Basically, we currently see 3 complementary categories of GCC plugins, >> > depending >> > on the nature of the extension: production, experimentation/research, and >> > new pass >> > integration. Each category naturally calls for slightly different API >> > features. >> > >> > >> I am not sure of the relevance of the "new pass integration plugins" >> examplified by MELT. >> [on the other hand, I do know Grigori and I believe he thought quite a >> lot about plugins, which I didn't. I only implemented one particular >> plugin machinery = MELT, knowing well that my approach is quite peculiar >> both in its goals and its implementation. I never thought of MELT as a >> universal plugin machinery). >> >> In my view, MELT fits quite precisely in the "production plugins" >> definition, while indeed I expect it to be useful mostly for >> "experimental/research" plugins. >> >> In my view also, the "new pass integration plugin" category should not >> really exist, because it probably can fit inside one (or both) of the >> above categories. >> >> MELT definitely claims to fit into the "production plugins" slot, >> because MELT always was concerned by efficiency and most importantly >> close integration of GCC internal structures. The major point of MELT >> is its several idioms to fit into the *evolving* GCC internals API, >> and I claim that the various MELT idioms (see my GROW paper) make >> close integration into GCC internals possible, and perhaps even easy >> (for each internal "feature" of GCC, it is really easy to code the >> couple of M
Re: [plugins] Comparison of plugin mechanisms
Grigori Fursin wrote: Dear all, Zbigniew and I prepared a page on GCC Wiki comparing several current plugin mechanisms (some parts should be updated) with some suggestions to move forward: http://gcc.gnu.org/wiki/GCC_PluginComparison In case we mixed up or misunderstood something about other plugin efforts, update this page, please ... Basically, we currently see 3 complementary categories of GCC plugins, depending on the nature of the extension: production, experimentation/research, and new pass integration. Each category naturally calls for slightly different API features. I respectfully disagree. I think this division is unnatural. Most plugins will be extending GCC with new features and there is a possibility of those plugin eventually becoming part of the GCC core. I don't see any difference between our "production" plugins, experimental plugins and new pass plugins. There are use-cases where our "production" plugins would want to rearrange pass execution and even to add new passes. I think something like the pass manager APIs you suggest should go into tree-pass.h. It doesn't make sense to put every new API that plugins would want to use into plugin headers as eventually some GCC code is likely to benefit from the same features. I don't know enough about feature stuff, but it seems like that'd be something that should also be a separate API from plugins. I don't know enough about the intent of the event stuff, but it sounds like that might actually be something only plugins would care about. Considering that there are already communities behind "production" and "experimental" plugins, we think that it would be better to merge two. We will try to prepare a small patch to support "experimental" plugins by the beginning of next week. In the mean time, would like to know your thoughts on that matter and how should we proceed forward !.. Like I said, I don't think the communities are as distinct as you are implying. Cheers, Taras
RE: [plugins] Comparison of plugin mechanisms
Hi Le-chun and Taras, You are right that instead of categorizing the plugin API into "production", "research", etc, we can just introduce several layers of abstraction that will be useful for different purposes such as quick prototyping or tightly coupled plugins, etc. We will update the Wiki to reflect that and avoid misunderstandings ... Zbigniew and I are preparing a patch that will include a higher abstraction layer that will be relying on the lower-level abstraction you documented (we just need to clean it now and we should be able to do it by the end of this week). We also provide high-level routines for pass manipulation since it's also critical for us and our current users - we can already add new code analysis/instrumentation passes or select/de-select/reorder existing passes easily - we will just need to synchronize on that implementation altogether to avoid duplicate work ... Will keep in touch, Grigori > -Original Message- > From: Le-Chun Wu [mailto:l...@google.com] > Sent: Wednesday, February 18, 2009 1:35 AM > To: Grigori Fursin > Cc: Basile STARYNKEVITCH; Diego Novillo; gcc@gcc.gnu.org; Sean Callanan; > Taras Glek; Gerald > Pfeifer; Zbigniew Chamski; Cupertino Miranda > Subject: Re: [plugins] Comparison of plugin mechanisms > > Hi Grigori and Zbigniew, > > I just took a look at the wiki page you guys put together. While I > have some reservations about categorizing the plugin APIs into > "production", "research", and "new pass", I totally agreed with you > that the plugin support should be implemented in layers, starting from > the core support that provides basic APIs and exposes all the internal > GCC data structures, to more complicated support that allows more > abstract APIs and maybe drastic changes in GCC compilation pipeline. > > I agree with Basile that the support for pass management should not be > in a different category. I think allowing plugin modules to hook new > passes into GCC is an important requirement for plugins to be really > useful, so we should provide basic pass management support (such as > adding a new pass or replacing an existing pass) even in the core (or > "production") APIs. And in the advanced/extended (or "research") APIs, > we can allow the whole pass manager to be replaced (probably like what > you did in ICI). In our plugin prototype, we have implemented basic > support for adding new passes and defined APIs that allow the plugin > writers to specify where/when to hook in the new passes. I will modify > the GCC Plugin API wiki with our proposal so that people can comment > on it. > > You mentioned that you will be sending out a patch this week. Will > your patch contains the implementation for both the "production" and > "research" APIs? If your patch contains only the research APIs, we can > prepare an official patch for the "production" APIs (based on the > patch that I send out a while back ago) and send it out for review > this week (if no one else would like to do it, that is). > > Thanks, > > Le-chun > > > On Fri, Feb 13, 2009 at 1:26 AM, Grigori Fursin > wrote: > > Hi Basile et al, > > > > Thanks a lot for detailed explanations. > > > > In addition to Zbigniew's reply: > > I didn't specifically put MELT into new category (I actually just > > updated the page and added it to the other categories too), > > but I still keep the new pass integration plugin, since > > some of our colleagues would like to add new passes including > > new optimizations, but have difficulties to do that since there > > is no full documented API, they may not want to write it in C > > (as you mentioned) and they are concerned about support overheads > > if the internal API will be changing all the time. Providing > > standard API would be of great help for such cases, but it rises > > other issues such as opening GCC to proprietary plugins, etc > > which has been heavily discussed here for some time. So I personally > > think we should leave it for the future and just implement minimalistic > > Plugin API that will already make many current and prospective users > > happy and then we can gradually extend it ... > > > > By the way, Sean, what do you think about the proposed APIs? > > Will it suit your purposes or some changes are also required? > > I would like to add info about your plugin system but just > > don't have much knowledge about that ... > > > > Cheers, > > Grigori > > > >> -Original Message- > >> From: Basile STARYNKEVITCH [mailto:bas...@starynkevitch.net] > >> Sent: Friday, February 13, 2009 8:38 AM > >> To: Grigori Fursin > >> Cc: 'Diego Novillo'; gcc@gcc.gnu.org; 'Sean Callanan'; 'Taras Glek'; > >> 'Le-Chun Wu'; 'Gerald > >> Pfeifer'; 'Zbigniew Chamski'; 'Cupertino Miranda' > >> Subject: Re: [plugins] Comparison of plugin mechanisms > >> > >> Hello All, > >> > >> > >> Grigori Fursin wrote: > >> > Basically, we currently see 3 complementary categories of GCC plugins, > >> > depending > >> > on the nature of the extension
Re: [Ada] Wrong code in gcc/ada/a-teioed.adb causing FAIL of ACATS cxf3a01 on mipsel and ia64
> Is the following okay to commit if it passes testing? OK for stage 1 (GCC 4.5), currently pretty much everything is frozen on mainline, except regressions (I hope stage 1 will open soon, since we have monthes of backlog of various fixes and new development blocked right now which will be painful to merge). Thanks. Arno
Re: [Ada] Wrong code in gcc/ada/a-teioed.adb causing FAIL of ACATS cxf3a01 on mipsel and ia64
> OK for stage 1 (GCC 4.5), currently pretty much everything is frozen on > mainline, except regressions (I hope stage 1 will open soon, since we have > monthes of backlog of various fixes and new development blocked right now > which will be painful to merge). If the ACATS test fails on ia64-linux, that's a regression from 4.3.x. -- Eric Botcazou
Re: [Ada] Wrong code in gcc/ada/a-teioed.adb causing FAIL of ACATS ?cxf3a01 on mipsel and ia64
> > OK for stage 1 (GCC 4.5), currently pretty much everything is frozen on > > mainline, except regressions (I hope stage 1 will open soon, since we have > > monthes of backlog of various fixes and new development blocked right now > > which will be painful to merge). > > If the ACATS test fails on ia64-linux, that's a regression from 4.3.x. OK, that's a little surprising, but if it's indeed a confirmed regression, then it's fine to commit now. Arno