Re: question about attribute aligned for functions
* Martin Sebor: > At the same time, the following passes on x86_64: > > __attribute__ ((aligned (1))) void f1 (void) { } > _Static_assert (__alignof__ (f1) == 1); // wrong alignof result > > __attribute__ ((aligned)) void f0 (void) { } > _Static_assert (__alignof__ (f0) == 16); > > __attribute__ ((aligned (2))) void f2 (void) { } > _Static_assert (__alignof__ (f2) == 2); Is there any value in implementing alignof on functions? sizeof (f1) is defined to be 1, allegedly to support function pointer arithmetic (which is why sizeof (void) is 1 as well). In practice, this is confusing to the programmer because the size of a function is at least known to the linker. Thanks, Florian
Re: GCC Common-Function-Attributes/const
On Tue, Nov 27, 2018 at 12:37 AM Martin Sebor wrote: > > On 11/26/18 1:30 PM, cmdLP #CODE wrote: > > Dear GCC-developer team, > > > > The specification of the const-attribute is a bit ambiguous, it does not > > fully specify which global variables are allowed to be read from. Obviously > > constant global compile-time initialized variables can be accessed without > > problem. But what about run time initialized variables like a lookup table > > or the initialization of an api? Does GCC may invoke the function before > > the initialization of the lookup table/api; or is it guaranteed, that the > > function is only called in places, where it would be called without the > > attribute. I also propose some additional attributes to let the programmer > > clarify his/her intentions. > > The purpose of the const attribute is to let GCC assume that > invocations of a function it's on with the same argument values > yield the same result no matter when they are made. Any function > that satisfies this constraint can be declared with the attribute, > whether it depends on values of global objects or not, const or > not, just as long as their values do not change between any two > invocations of the function in a way that would affect its result. > > It should even be possible to define a const function to return > the value of non-constant dynamically initialized data. For > instance: > >__attribute__ ((const)) int prime (int i) >{ > static int primes[N]; > if (!primes[0]) >// fill primes with prime numbers > return primes[i]; >} > > This is a valid const function because its result depends only > on the value of its argument. > > The documentation for the attribute is being improved as we speak > in response to PR79738. I expect it to take a few more tweaks > before we're completely happy with it. Note that the compiler, when you annotate a function with const, may happily re-order a call to such function with the runtime initialization sequence. So in this context it's only safe to use the const attribution in case the compiler cannot possibly fall into this "trap". C++ constructors executed as part of global object initialization is probably safe. > > Here are some code snippets, how GCC might change the generated code, > > according to the documentation. > > In my opinion, each generated output should be assigned to a distinct > > attribute. > > > > *Function declarations:* > > *void init_values();* > > *int read_value(int index) [[gnu::const]];* > > > > *void use_value(int value);* > > > > *Problematic code:* > > *int main(int argc, char** argv) {* > > *init_values();* > > *for(int i=1; i < argc; ++i) {* > > *int value = read_value(0); // constant* > > *use_value(value, argv[i]);* > > *}* > > *}* > > > > Here are some possible outputs, which are possible, because of the > > ambiguity of the documentation. The attribute next to "Transformation" is > > the proposed new attribute names, which could lead to the transformatio > > (when [[gnu::const]] is replaced with it). > > *Transformation [[gnu::const, gnu::is_simple]]* > > *int read_value__with_0;* > > > > *int main(int argc, char**) {* > > *read_value__with_0 = read_value(0);* > > *init_values();* > > *for(int i=1; i < argc; ++i) {* > > *use_value(read_value__with_0, argv[i]);* > > *}* > > *}* > > > > The code is called at some undefined point, maybe at the start of main, > > even if it is never used, because the compiler assumes, that the function > > simply reads from constant memory/does a simple calculation. In the case of > > the example, it is not what the programmer intended. > > Right. This definition makes read_value() unsuitable for attribute > const. > > I don't have a sense of how much software would benefit from > the attributes proposed below. > > Martin > > > > > *Transformation [[gnu::const]]* > > *int read_value__with_0;* > > > > *int main(int argc, char** argv) {* > > *if(argc > 1) read_value__with_0 = read_value(0);* > > *init_values();* > > *for(int i=1; i < argc; ++i) {* > > *use_value(read_value__with_0, argv[i]);* > > *}* > > *}* > > > > This is almost the same to the previous version, but it only calls the > > function, if the result is used. > > > > Both attributes (the time when it is called is undefined) can also result > > in the following code where the attributes give a stronger guarantee, when > > it is called (the first time). The following works semantically as the > > programmer intended/when the attributes were ignored. > > > > *Transformation** [[gnu::const(init_values()), gnu::is_simple]]* > > *int read_value__with_0;* > > *int main() {* > > *init_values();* > > *read_value__with_0 = read_value(0);* > > *for(int i=1; i < 100; ++i) {* > > *use_value(read_value__with_0, argv[i]);* > > *}* > > *}* > > > > *Transformation **[[gnu::const(init_values())]]* > > *int read_value__with_0;* >
Re: question about attribute aligned for functions
On 11/28/18 6:04 AM, Florian Weimer wrote: * Martin Sebor: At the same time, the following passes on x86_64: __attribute__ ((aligned (1))) void f1 (void) { } _Static_assert (__alignof__ (f1) == 1); // wrong alignof result __attribute__ ((aligned)) void f0 (void) { } _Static_assert (__alignof__ (f0) == 16); __attribute__ ((aligned (2))) void f2 (void) { } _Static_assert (__alignof__ (f2) == 2); Is there any value in implementing alignof on functions? sizeof (f1) is defined to be 1, allegedly to support function pointer arithmetic (which is why sizeof (void) is 1 as well). In practice, this is confusing to the programmer because the size of a function is at least known to the linker. I also don't see any utility in applying sizeof to functions, at least not with the existing semantics of evaluating to 1. alignof seems somewhat more useful, but not tremendously so, for similar reasons (at least it gives the lower bound). But both have been implemented for ages so they couldn't be removed without some risk. I don't know if the benefits of the removal would justify the risks. Martin
Gimpel lowering question.
I have a small test case which generates poor quality code on my target. Here is the original: if (cond1 == 2048 || cond2 == 8) { x = x + y; } return x; This ends up generating a series of instructions to compute a flag with the result of the condition followed by a single compare with zero and a jump. Better code would be two compares and two jumps. The gimple is _1 = cond1 == 2048; _2 = cond2 == 8; _3 = _1 | _2; if (_3 != 0) goto ; else goto ; ... so this poor code sequence is essentially a correct translation. On MIPS, for the same test case, the gimple is different: if (cond1 == 2048) goto ; else goto ; : if (cond2 == 8) goto ; else goto ; : ... which generates the better code sequence. Can someone give me a pointer where to find where the lowering pass decides to break up a condition into multiple tests? Is there a target configuration option which I have overlooked or maybe set incorrectly? -- Michael Eagerea...@eagerm.com 1960 Park Blvd., Palo Alto, CA 94306
Re: Gimpel lowering question.
On 11/28/18 10:00 AM, Michael Eager wrote: > I have a small test case which generates poor quality code on my target. > Here is the original: > > if (cond1 == 2048 || cond2 == 8) > { > x = x + y; > } > return x; > > This ends up generating a series of instructions to compute a flag with > the result of the condition followed by a single compare with zero and > a jump. Better code would be two compares and two jumps. > > The gimple is > > _1 = cond1 == 2048; > _2 = cond2 == 8; > _3 = _1 | _2; > if (_3 != 0) goto ; else goto ; > ... > > so this poor code sequence is essentially a correct translation. > > On MIPS, for the same test case, the gimple is different: > > if (cond1 == 2048) goto ; else goto ; > : > if (cond2 == 8) goto ; else goto ; > : > ... > > which generates the better code sequence. > > Can someone give me a pointer where to find where the lowering > pass decides to break up a condition into multiple tests? Is > there a target configuration option which I have overlooked or > maybe set incorrectly? BRANCH_COST, which comes into play during generation of the initial trees as well in passes which try to optimize branchy code into straighter code. jeff
Re: [GSOC] variations in testsuite results
Hi Siddhartha, On Tue, Nov 27 2018, Siddhartha Sen wrote: > I am Siddhartha Sen,currently pursuing my B.Tech degree in Information > Science and Engineering,2nd year. I have taken a keen interest in your > projects and have some ideas of my own as well. I am really interested in > working with you in G-SOC 2019. We are delighted. > I am proficient in C and C++ and am eager > to work on brushing my skills in whichever field required. I have already > checked out the GCC trunk source-code and am being able to build GCC from > it.I have run the testsuite and saved the results. Great, those are all the essential first steps, it seems like you are ready to work on your first patch. > However, upon building > it and saving it again, the results don't match. I apologise for my lack of > knowledge but any help on your part would be great. How did you compare the test results and which tests behaved differently? How did you configure GCC? Was it on an x86_64? Unfortunately, this sometimes happens. As far as I can tell, the libgo testsuite is very flaky and I basically ignore its results. In the past, I believe that also some guality tests sometimes passed and sometimes did not, but I have not heard such reports recently. These tests however also depend on the GDB (version) you have installed on your system. Martin
Re: Gimpel lowering question.
On 11/28/18 09:10, Jeff Law wrote: On 11/28/18 10:00 AM, Michael Eager wrote: I have a small test case which generates poor quality code on my target. Here is the original: if (cond1 == 2048 || cond2 == 8) { x = x + y; } return x; This ends up generating a series of instructions to compute a flag with the result of the condition followed by a single compare with zero and a jump. Better code would be two compares and two jumps. The gimple is _1 = cond1 == 2048; _2 = cond2 == 8; _3 = _1 | _2; if (_3 != 0) goto ; else goto ; ... so this poor code sequence is essentially a correct translation. On MIPS, for the same test case, the gimple is different: if (cond1 == 2048) goto ; else goto ; : if (cond2 == 8) goto ; else goto ; : ... which generates the better code sequence. Can someone give me a pointer where to find where the lowering pass decides to break up a condition into multiple tests? Is there a target configuration option which I have overlooked or maybe set incorrectly? BRANCH_COST, which comes into play during generation of the initial trees as well in passes which try to optimize branchy code into straighter code. Thanks. I did look at BRANCH_COST, and played with the values, but it didn't seem to have any affect. I'll take a closer look. -- Michael Eagerea...@eagerm.com 1960 Park Blvd., Palo Alto, CA 94306
Re: Gimpel lowering question.
On 11/28/18 10:47 AM, Michael Eager wrote: > On 11/28/18 09:10, Jeff Law wrote: >> On 11/28/18 10:00 AM, Michael Eager wrote: >>> I have a small test case which generates poor quality code on my target. >>> Here is the original: >>> >>> if (cond1 == 2048 || cond2 == 8) >>> { >>> x = x + y; >>> } >>> return x; >>> >>> This ends up generating a series of instructions to compute a flag with >>> the result of the condition followed by a single compare with zero and >>> a jump. Better code would be two compares and two jumps. >>> >>> The gimple is >>> >>> _1 = cond1 == 2048; >>> _2 = cond2 == 8; >>> _3 = _1 | _2; >>> if (_3 != 0) goto ; else goto ; >>> ... >>> >>> so this poor code sequence is essentially a correct translation. >>> >>> On MIPS, for the same test case, the gimple is different: >>> >>> if (cond1 == 2048) goto ; else goto ; >>> : >>> if (cond2 == 8) goto ; else goto ; >>> : >>> ... >>> >>> which generates the better code sequence. >>> >>> Can someone give me a pointer where to find where the lowering >>> pass decides to break up a condition into multiple tests? Is >>> there a target configuration option which I have overlooked or >>> maybe set incorrectly? >> BRANCH_COST, which comes into play during generation of the initial >> trees as well in passes which try to optimize branchy code into >> straighter code. > > Thanks. I did look at BRANCH_COST, and played with the values, but it > didn't seem to have any affect. I'll take a closer look. I'd start by looking at the state the trees during gimplification then walk forward or backward as necessary. Jeff
write w/o approval policy (Re: [PATCH] clarify comments for implicit_p flag for built-ins)
On 11/28/18 6:35 AM, Richard Biener wrote: On Tue, Nov 27, 2018 at 3:52 AM Martin Sebor wrote: Ping: https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01759.html If there are no objections or suggestions for tweaks I'll commit this updated comment this week. Please do not commit such changes w/o approval. Since you're the second maintainer to ask me that in response to a patch to update comments I'd like to get some clarity here. I have been assuming that the GCC Write access policy (quoted below) lets those of us with write-after-approval make a judgment call as to when a change is sufficiently safe to commit: Obvious fixes can be committed without prior approval. Just check in the fix and copy it to gcc-patches. A good test to determine whether a fix is obvious: "will the person who objects to my work the most be able to find a fault with my fix?" If the fix is later found to be faulty, it can always be rolled back. We don't want to get overly restrictive about checkin policies. (https://www.gnu.org/software/gcc/svnwrite.html#policies) If we are not at liberty to make this judgment call in even the most innocuous cases like comments, when does this policy actually apply? (It should be updated to make it clear.) Thanks Martin
Re: Gimpel lowering question.
On Wed, Nov 28, 2018 at 9:47 AM Michael Eager wrote: > > On 11/28/18 09:10, Jeff Law wrote: > > On 11/28/18 10:00 AM, Michael Eager wrote: > >> I have a small test case which generates poor quality code on my target. > >> Here is the original: > >> > >>if (cond1 == 2048 || cond2 == 8) > >> { > >>x = x + y; > >> } > >>return x; > >> > >> This ends up generating a series of instructions to compute a flag with > >> the result of the condition followed by a single compare with zero and > >> a jump. Better code would be two compares and two jumps. > >> > >> The gimple is > >> > >>_1 = cond1 == 2048; > >>_2 = cond2 == 8; > >>_3 = _1 | _2; > >>if (_3 != 0) goto ; else goto ; > >> ... > >> > >> so this poor code sequence is essentially a correct translation. > >> > >> On MIPS, for the same test case, the gimple is different: > >> > >>if (cond1 == 2048) goto ; else goto ; > >>: > >>if (cond2 == 8) goto ; else goto ; > >>: > >> ... > >> > >> which generates the better code sequence. > >> > >> Can someone give me a pointer where to find where the lowering > >> pass decides to break up a condition into multiple tests? Is > >> there a target configuration option which I have overlooked or > >> maybe set incorrectly? > > BRANCH_COST, which comes into play during generation of the initial > > trees as well in passes which try to optimize branchy code into > > straighter code. > > Thanks. I did look at BRANCH_COST, and played with the values, but it > didn't seem to have any affect. I'll take a closer look. Look at LOGICAL_OP_NON_SHORT_CIRCUIT . By defualt, it is defined as: (BRANCH_COST (optimize_function_for_speed_p (cfun), \ false) >= 2) But MIPS defines it as 0. Changing LOGICAL_OP_NON_SHORT_CIRCUIT to 0 will disable this optimization. LOGICAL_OP_NON_SHORT_CIRCUIT controls both places where (cond1 == 2048 || cond2 == 8) would remove the branch. NOTE I think MIPS defines LOGICAL_OP_NON_SHORT_CIRCUIT incorrectly but that is a different story. Thanks, Andrew Pinski > > -- > Michael Eagerea...@eagerm.com > 1960 Park Blvd., Palo Alto, CA 94306
Re: write w/o approval policy (Re: [PATCH] clarify comments for implicit_p flag for built-ins)
On 11/28/18 11:39 AM, Martin Sebor wrote: > On 11/28/18 6:35 AM, Richard Biener wrote: >> On Tue, Nov 27, 2018 at 3:52 AM Martin Sebor wrote: >>> >>> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01759.html >>> >>> If there are no objections or suggestions for tweaks I'll commit >>> this updated comment this week. >> >> Please do not commit such changes w/o approval. > > Since you're the second maintainer to ask me that in response > to a patch to update comments I'd like to get some clarity here. > > I have been assuming that the GCC Write access policy (quoted > below) lets those of us with write-after-approval make a judgment > call as to when a change is sufficiently safe to commit: > > Obvious fixes can be committed without prior approval. Just > check in the fix and copy it to gcc-patches. A good test to > determine whether a fix is obvious: "will the person who > objects to my work the most be able to find a fault with my > fix?" If the fix is later found to be faulty, it can always > be rolled back. We don't want to get overly restrictive about > checkin policies. > > (https://www.gnu.org/software/gcc/svnwrite.html#policies) > > If we are not at liberty to make this judgment call in even > the most innocuous cases like comments, when does this policy > actually apply? (It should be updated to make it clear.) The thing is I looked at the patch and it was far from obvious what was going on. Thus I put it in my queue of things to dig deeper into. I haven't done that digging yet. Comments are actually important. They often describe what the code is supposed to do, rationale, historical context, etc. Just because we're changing a comment doesn't mean it's inherently trivial/obvious. I'm generally supportive of lessening friction for developers and I welcome proposals to do that. Jeff
Re: question about attribute aligned for functions
On 11/27/18 11:57 AM, Martin Sebor wrote: > On 11/26/18 3:37 PM, Jeff Law wrote: >> On 11/23/18 12:31 PM, Martin Sebor wrote: >>> GCC currently accepts the declaration of f0 below but ignores >>> the attribute. On aarch64 (and I presume on other targets with >>> a default function alignment greater than 1), GCC rejects f1 >>> with an error, even though it accepts -falign-functions=1 >>> without as much as a warning. >>> >>> Clang, on the other hand, rejects f0 with a hard error because >>> the alignment is not a power of two, but accepts f1 and appears >>> to honor the attribute. It also accepts -falign-functions=1. >>> >>> I think diagnosing f0 with a warning is helpful because an explicit >>> zero alignment is most likely a mistake (especially when it comes >>> from a macro or some computation). >>> >>> But I don't see a good reason to reject a program that specifies >>> a smaller alignment for a function when the default (or minimum) >>> alignment is greater. A smaller alignment is trivially satisfied >>> by a greater alignment so either accepting it or dropping it seems >>> preferable to failing with an error (either could be with or without >>> a warning). >>> >>> __attribute__ ((aligned (0))) void f0 (void); // accepted, ignored >>> __attribute__ ((aligned (1))) void f1 (void); // aarch64 error >>> __attribute__ ((aligned (4))) void f4 (void); // okay >>> >>> Does anyone know why GCC rejects the program, or can anyone think >>> of a reason why GCC should not behave as suggested above? >> Note we have targets that support single byte opcodes and do not have >> any requirements for functions starting on any boundary. mn103 comes to >> mind. >> >> However, the attribute can't be used to decrease a function's alignment, >> so values of 0 or 1 are in practice totally uninteresting and one could >> make an argument to warn for them. > > The attribute does reduce the default alignment at least on some > targets. For instance, on x86_64 it lowers it from the default > 16 to as little as 2, but it silently ignores 1. [ ... ] You cannot use this attribute to decrease the alignment of a function, only to increase it. However, when you explicitly specify a function alignment this overrides the effect of the @option{-falign-functions} (@pxref{Optimize Options}) option for this function. [ ... ] My reading of that would be that I would get an error/warning if I even specified an alignment attribute which decreased the alignment. If it instead said something like You can not rely on this attribute to decrease ... Then current behavior (where apparently you can decrease the alignment on some ports) makes much more sense. I guess it ultimately depends on how one interprets that tidbit from the docs. >> >> Whether or not to warn in general if the alignment attribute is smaller >> than the default may be subject to debate. I guess it depends on the >> general intent that we'd find in real world codes. > > I would expect real world code to care about achieving at least > the specified alignment. If we only allow increasing, yes. At which point what do the values 0 or 1 realistically mean? If we allow decreasing, then the user may be asking for a smaller alignment to achieve better code density. > > A less restrictive alignment requirement is satisfied by providing > a more restrictive one, and (the above notwithstanding) the manual > documents that > > You cannot use this attribute to decrease the alignment of > a function, only to increase it. > > So I wouldn't expect real code to be relying on decreasing > the alignment. > > That said, since GCC does make it possible to decrease the default > alignment of functions, I can think of no reason for it not to > continue when it's possible. I.e., on targets with underaligned > instruction reads to be honor requests for underaligned functions. > On strictly aligned targets I think the safe thing to do is to > quietly ignore requests for smaller alignments by default, and > perhaps provide a new option to request a warning (with the warning > being disabled by default). > > Do you see a problem with this approach? ISTM we need to figure out whether or not we consider an attempt to lower the alignment as invalid vs we'll try, but no guarantees. jeff