Re: A couple GIMPLE questions
>That's not a question? Are you asking why PHIs exist at all? >They are the standard way to represent merging in SSA >representations. You can iterate on the PHIs of a basic block, etc. Marc, I first worked with the SSA form twenty years ago so yes I am aware of what a phi is... I've just never seen a compiler eliminate an assignment of a variable to a constant and jam the constant into the phi where the SSA variable should be. What a phi is all about is representing data flow and a constant in the phi doesn't seem to be related to that. I can deal with this but it seems that having to crawl the phis looking for constants seems baroque. I would hope there is a control that can suppress this or a transformation that I can invoke to reverse it... Thanks, Gary From: Marc Glisse Sent: Saturday, September 5, 2020 11:29 PM To: Gary Oblock Cc: gcc@gcc.gnu.org Subject: Re: A couple GIMPLE questions [EXTERNAL EMAIL NOTICE: This email originated from an external sender. Please be mindful of safe email handling and proprietary information protection practices.] On Sat, 5 Sep 2020, Gary Oblock via Gcc wrote: > First off one of the questions just me being curious but > second is quite serious. Note, this is GIMPLE coming > into my optimization and not something I've modified. > > Here's the C code: > > type_t * > do_comp( type_t *data, size_t len) > { > type_t *res; > type_t *x = min_of_x( data, len); > type_t *y = max_of_y( data, len); > > res = y; > if ( x < y ) res = 0; > return res; > } > > And here's the resulting GIMPLE: > > ;; Function do_comp.constprop (do_comp.constprop.0, funcdef_no=5, > decl_uid=4392, cgraph_uid=3, symbol_order=68) (executed once) > > do_comp.constprop (struct type_t * data) > { > struct type_t * res; > struct type_t * x; > struct type_t * y; > size_t len; > > [local count: 1073741824]: > > [local count: 1073741824]: > x_2 = min_of_x (data_1(D), 1); > y_3 = max_of_y (data_1(D), 1); > if (x_2 < y_3) >goto ; [29.00%] > else >goto ; [71.00%] > > [local count: 311385128]: > > [local count: 1073741824]: > # res_4 = PHI > return res_4; > > } > > The silly question first. In the "if" stmt how does GCC > get those probabilities? Which it shows as 29.00% and > 71.00%. I believe they should both be 50.00%. See the profile_estimate pass dump. One branch makes the function return NULL, which makes gcc guess that it may be a bit less likely than the other. Those are heuristics, which are tuned to help on average, but of course they are sometimes wrong. > The serious question is what is going on with this phi? >res_4 = PHI > > This makes zero sense practicality wise to me and how is > it supposed to be recognized and used? Note, I really do > need to transform the "0B" into something else for my > structure reorganization optimization. That's not a question? Are you asking why PHIs exist at all? They are the standard way to represent merging in SSA representations. You can iterate on the PHIs of a basic block, etc. > CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is > for the sole use of the intended recipient(s) and contains information that > is confidential and proprietary to Ampere Computing or its subsidiaries. It > is to be used solely for the purpose of furthering the parties' business > relationship. Any unauthorized review, copying, or distribution of this email > (or any attachments thereto) is strictly prohibited. If you are not the > intended recipient, please contact the sender immediately and permanently > delete the original and any copies of this email and any attachments thereto. Could you please get rid of this when posting on public mailing lists? -- Marc Glisse
Re: A couple GIMPLE questions
>Could you please get rid of this when posting on public mailing lists? No, I have no control over that but I'll give the email of our corporate IT if you want to complain to them... From: Marc Glisse Sent: Saturday, September 5, 2020 11:29 PM To: Gary Oblock Cc: gcc@gcc.gnu.org Subject: Re: A couple GIMPLE questions [EXTERNAL EMAIL NOTICE: This email originated from an external sender. Please be mindful of safe email handling and proprietary information protection practices.] On Sat, 5 Sep 2020, Gary Oblock via Gcc wrote: > First off one of the questions just me being curious but > second is quite serious. Note, this is GIMPLE coming > into my optimization and not something I've modified. > > Here's the C code: > > type_t * > do_comp( type_t *data, size_t len) > { > type_t *res; > type_t *x = min_of_x( data, len); > type_t *y = max_of_y( data, len); > > res = y; > if ( x < y ) res = 0; > return res; > } > > And here's the resulting GIMPLE: > > ;; Function do_comp.constprop (do_comp.constprop.0, funcdef_no=5, > decl_uid=4392, cgraph_uid=3, symbol_order=68) (executed once) > > do_comp.constprop (struct type_t * data) > { > struct type_t * res; > struct type_t * x; > struct type_t * y; > size_t len; > > [local count: 1073741824]: > > [local count: 1073741824]: > x_2 = min_of_x (data_1(D), 1); > y_3 = max_of_y (data_1(D), 1); > if (x_2 < y_3) >goto ; [29.00%] > else >goto ; [71.00%] > > [local count: 311385128]: > > [local count: 1073741824]: > # res_4 = PHI > return res_4; > > } > > The silly question first. In the "if" stmt how does GCC > get those probabilities? Which it shows as 29.00% and > 71.00%. I believe they should both be 50.00%. See the profile_estimate pass dump. One branch makes the function return NULL, which makes gcc guess that it may be a bit less likely than the other. Those are heuristics, which are tuned to help on average, but of course they are sometimes wrong. > The serious question is what is going on with this phi? >res_4 = PHI > > This makes zero sense practicality wise to me and how is > it supposed to be recognized and used? Note, I really do > need to transform the "0B" into something else for my > structure reorganization optimization. That's not a question? Are you asking why PHIs exist at all? They are the standard way to represent merging in SSA representations. You can iterate on the PHIs of a basic block, etc. > CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is > for the sole use of the intended recipient(s) and contains information that > is confidential and proprietary to Ampere Computing or its subsidiaries. It > is to be used solely for the purpose of furthering the parties' business > relationship. Any unauthorized review, copying, or distribution of this email > (or any attachments thereto) is strictly prohibited. If you are not the > intended recipient, please contact the sender immediately and permanently > delete the original and any copies of this email and any attachments thereto. Could you please get rid of this when posting on public mailing lists? -- Marc Glisse
Function signatures in extern "C".
Hi g++.dg/abi/guard3.C has: extern "C" int __cxa_guard_acquire(); Which might not be a suitable declaration, depending on how the ‘extern “C”’ is supposed to affect the function signature generated. IF, the extern C should make this parse as a “K&R” style function - then the TYPE_ARG_TYPES should be NULL (and the testcase is OK). However, we are parsing the decl as int __cxa_guard_acquire(void) (i.e. C++ rules on the empty parens), which makes the testcase not OK. This means that the declaration is now misleading (and it’s just luck that expand_call happens to count the length of the TYPE_ARG_TYPES list without looking to see what the types are) - in this case it happens to work out from this luck - since there’s only one arg so the length of the void args list agrees with what we want. —— So .. the question is “which is wrong, the test-case or the assignment of the TYPE_ARG_TYPES”? [we can’t easily diagnose this at this point, but I do have a patch to diagnose the case where we pass a void-list to expand_call and then try to expand a call to the callee with an inappropriate set of parms] (it’s trivial to fix the test-case as extern "C" int __cxa_guard_acquire(__UINT64_TYPE__ *);, I guess) thanks Iain
Re: A couple GIMPLE questions
On September 6, 2020 9:38:45 AM GMT+02:00, Gary Oblock via Gcc wrote: >>That's not a question? Are you asking why PHIs exist at all? >>They are the standard way to represent merging in SSA >>representations. You can iterate on the PHIs of a basic block, etc. > >Marc, > >I first worked with the SSA form twenty years ago so yes I am >aware of what a phi is... I've just never seen a compiler eliminate >an assignment of a variable to a constant and jam the constant into >the phi where the SSA variable should be. What a phi is all about >is representing data flow and a constant in the phi doesn't seem >to be related to that. I can deal with this but it seems that having to >crawl the phis looking for constants seems baroque. I would hope >there is a control that can suppress this or a transformation >that I can invoke to reverse it... No, there isn't. We happily propagate constants into PHI nodes. Richard. > >Thanks, > >Gary > >From: Marc Glisse >Sent: Saturday, September 5, 2020 11:29 PM >To: Gary Oblock >Cc: gcc@gcc.gnu.org >Subject: Re: A couple GIMPLE questions > >[EXTERNAL EMAIL NOTICE: This email originated from an external sender. >Please be mindful of safe email handling and proprietary information >protection practices.] > > >On Sat, 5 Sep 2020, Gary Oblock via Gcc wrote: > >> First off one of the questions just me being curious but >> second is quite serious. Note, this is GIMPLE coming >> into my optimization and not something I've modified. >> >> Here's the C code: >> >> type_t * >> do_comp( type_t *data, size_t len) >> { >> type_t *res; >> type_t *x = min_of_x( data, len); >> type_t *y = max_of_y( data, len); >> >> res = y; >> if ( x < y ) res = 0; >> return res; >> } >> >> And here's the resulting GIMPLE: >> >> ;; Function do_comp.constprop (do_comp.constprop.0, funcdef_no=5, >decl_uid=4392, cgraph_uid=3, symbol_order=68) (executed once) >> >> do_comp.constprop (struct type_t * data) >> { >> struct type_t * res; >> struct type_t * x; >> struct type_t * y; >> size_t len; >> >> [local count: 1073741824]: >> >> [local count: 1073741824]: >> x_2 = min_of_x (data_1(D), 1); >> y_3 = max_of_y (data_1(D), 1); >> if (x_2 < y_3) >>goto ; [29.00%] >> else >>goto ; [71.00%] >> >> [local count: 311385128]: >> >> [local count: 1073741824]: >> # res_4 = PHI >> return res_4; >> >> } >> >> The silly question first. In the "if" stmt how does GCC >> get those probabilities? Which it shows as 29.00% and >> 71.00%. I believe they should both be 50.00%. > >See the profile_estimate pass dump. One branch makes the function >return >NULL, which makes gcc guess that it may be a bit less likely than the >other. Those are heuristics, which are tuned to help on average, but of >course they are sometimes wrong. > >> The serious question is what is going on with this phi? >>res_4 = PHI >> >> This makes zero sense practicality wise to me and how is >> it supposed to be recognized and used? Note, I really do >> need to transform the "0B" into something else for my >> structure reorganization optimization. > >That's not a question? Are you asking why PHIs exist at all? They are >the >standard way to represent merging in SSA representations. You can >iterate >on the PHIs of a basic block, etc. > >> CONFIDENTIALITY NOTICE: This e-mail message, including any >attachments, is for the sole use of the intended recipient(s) and >contains information that is confidential and proprietary to Ampere >Computing or its subsidiaries. It is to be used solely for the purpose >of furthering the parties' business relationship. Any unauthorized >review, copying, or distribution of this email (or any attachments >thereto) is strictly prohibited. If you are not the intended recipient, >please contact the sender immediately and permanently delete the >original and any copies of this email and any attachments thereto. > >Could you please get rid of this when posting on public mailing lists? > >-- >Marc Glisse
Re: Function signatures in extern "C".
On Sun, 6 Sep 2020 at 16:23, Iain Sandoe wrote: > > Hi > > g++.dg/abi/guard3.C > > has: > > extern "C" int __cxa_guard_acquire(); > > Which might not be a suitable declaration, depending on how the ‘extern > “C”’ is supposed to affect the function signature generated. > > IF, the extern C should make this parse as a “K&R” style function - then > the TYPE_ARG_TYPES should be NULL (and the testcase is OK). > > However, we are parsing the decl as int __cxa_guard_acquire(void) (i.e. C++ > rules on the empty parens), which makes the testcase not OK. That is the correct parse. Using extern "C" doesn't mean the code is C, it only affects mangling. It still has to follow C++ rules. In practice you can still link to the definition, because its name is just "__cxa_guard_acquire" irrespective of what parameter list is present in the declaration. > This means that the declaration is now misleading (and it’s just luck that > expand_call happens to count the length of the TYPE_ARG_TYPES list without > looking to see what the types are) - in this case it happens to work out > from this luck - since there’s only one arg so the length of the void args > list agrees with what we want. > > —— > > So .. the question is “which is wrong, the test-case or the assignment of > the TYPE_ARG_TYPES”? > > [we can’t easily diagnose this at this point, but I do have a patch to > diagnose the case where we pass a void-list to expand_call and then try to > expand a call to the callee with an inappropriate set of parms] > > (it’s trivial to fix the test-case as extern "C" int > __cxa_guard_acquire(__UINT64_TYPE__ *);, I guess) But PR 45603 is ice-on-invalid triggered by the incorrect declaration of __cxa_guard_acquire. So the incorrect declaration is what originally reproduced the bug, and "fixing" it would make the test useless. It's probably worth adding a comment about that in the test. Maybe the test should give a compile-time error and XFAIL, but fixing the declaration doesn't seem right.
Re: Function signatures in extern "C".
Jonathan Wakely via Gcc wrote: On Sun, 6 Sep 2020 at 16:23, Iain Sandoe wrote: g++.dg/abi/guard3.C has: extern "C" int __cxa_guard_acquire(); Which might not be a suitable declaration, depending on how the ‘extern “C”’ is supposed to affect the function signature generated. IF, the extern C should make this parse as a “K&R” style function - then the TYPE_ARG_TYPES should be NULL (and the testcase is OK). However, we are parsing the decl as int __cxa_guard_acquire(void) (i.e. C++ rules on the empty parens), which makes the testcase not OK. That is the correct parse. Using extern "C" doesn't mean the code is C, it only affects mangling. It still has to follow C++ rules. In practice you can still link to the definition, because its name is just "__cxa_guard_acquire" irrespective of what parameter list is present in the declaration. Linking isn’t the problem in this case. The problem is that we arrive at “expand_call” with a function decl that says f(void) .. and a call parmeter list containing a pointer type. We happily pass the pointer in the place of the ‘void’ - because the code only counts the number of entries and there’s one - so it happens to work. .. that’s not true in the general case and for all calling conventions. (this is what I mean by it happens to work by luck below). This means that the declaration is now misleading (and it’s just luck that expand_call happens to count the length of the TYPE_ARG_TYPES list without looking to see what the types are) - in this case it happens to work out from this luck - since there’s only one arg so the length of the void args list agrees with what we want. —— So .. the question is “which is wrong, the test-case or the assignment of the TYPE_ARG_TYPES”? [we can’t easily diagnose this at this point, but I do have a patch to diagnose the case where we pass a void-list to expand_call and then try to expand a call to the callee with an inappropriate set of parms] (it’s trivial to fix the test-case as extern "C" int __cxa_guard_acquire(__UINT64_TYPE__ *);, I guess) But PR 45603 is ice-on-invalid triggered by the incorrect declaration of __cxa_guard_acquire. So the incorrect declaration is what originally reproduced the bug, and "fixing" it would make the test useless. Ah OK. It's probably worth adding a comment about that in the test. Yes - that would help (will add it to my TODO). Maybe the test should give a compile-time error and XFAIL, but fixing the declaration doesn't seem right. I guess (because the code is invalid) there’s not much motivation to make it more robust - e.g. diagnose the mismatch in the call(s) synthesized to __cxa_guard_acquire. It seems we only try to build these function decl(s) once - lazily - so that a wrong one will persist for the whole TU (and we don’t seem to check that the decl matches the itanium ABI - perhaps that’s intentional tho). cheers Iain
gcc-11-20200906 is now available
Snapshot gcc-11-20200906 is now available on https://gcc.gnu.org/pub/gcc/snapshots/11-20200906/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 11 git branch with the following options: git://gcc.gnu.org/git/gcc.git branch master revision 23f8b90c401842afcbaa50a7fd3c2f37818f4396 You'll find: gcc-11-20200906.tar.xz Complete GCC SHA256=ac017f84cb7ff42f8e7e6fea2bff8b45d586eb4333467c18daa2002a6f2265d0 SHA1=1f502e4603b081268d5440b02d53e6a3aadef587 Diffs from 11-20200830 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-11 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: Function signatures in extern "C".
GCC has an extension on machaines with cxx_implicit_extern_c (what used to be !NO_IMPLICIT_EXTERN_C). On such targets we'll treat 'extern "C" void Foo ()' as-if the argument list is variadic. (or something approximating that) perhaps that is confusing things? nathan On 9/6/20 4:43 PM, Iain Sandoe wrote: Jonathan Wakely via Gcc wrote: On Sun, 6 Sep 2020 at 16:23, Iain Sandoe wrote: g++.dg/abi/guard3.C has: extern "C" int __cxa_guard_acquire(); Which might not be a suitable declaration, depending on how the ‘extern “C”’ is supposed to affect the function signature generated. IF, the extern C should make this parse as a “K&R” style function - then the TYPE_ARG_TYPES should be NULL (and the testcase is OK). However, we are parsing the decl as int __cxa_guard_acquire(void) (i.e. C++ rules on the empty parens), which makes the testcase not OK. That is the correct parse. Using extern "C" doesn't mean the code is C, it only affects mangling. It still has to follow C++ rules. In practice you can still link to the definition, because its name is just "__cxa_guard_acquire" irrespective of what parameter list is present in the declaration. Linking isn’t the problem in this case. The problem is that we arrive at “expand_call” with a function decl that says f(void) .. and a call parmeter list containing a pointer type. We happily pass the pointer in the place of the ‘void’ - because the code only counts the number of entries and there’s one - so it happens to work. .. that’s not true in the general case and for all calling conventions. (this is what I mean by it happens to work by luck below). This means that the declaration is now misleading (and it’s just luck that expand_call happens to count the length of the TYPE_ARG_TYPES list without looking to see what the types are) - in this case it happens to work out from this luck - since there’s only one arg so the length of the void args list agrees with what we want. —— So .. the question is “which is wrong, the test-case or the assignment of the TYPE_ARG_TYPES”? [we can’t easily diagnose this at this point, but I do have a patch to diagnose the case where we pass a void-list to expand_call and then try to expand a call to the callee with an inappropriate set of parms] (it’s trivial to fix the test-case as extern "C" int __cxa_guard_acquire(__UINT64_TYPE__ *);, I guess) But PR 45603 is ice-on-invalid triggered by the incorrect declaration of __cxa_guard_acquire. So the incorrect declaration is what originally reproduced the bug, and "fixing" it would make the test useless. Ah OK. It's probably worth adding a comment about that in the test. Yes - that would help (will add it to my TODO). Maybe the test should give a compile-time error and XFAIL, but fixing the declaration doesn't seem right. I guess (because the code is invalid) there’s not much motivation to make it more robust - e.g. diagnose the mismatch in the call(s) synthesized to __cxa_guard_acquire. It seems we only try to build these function decl(s) once - lazily - so that a wrong one will persist for the whole TU (and we don’t seem to check that the decl matches the itanium ABI - perhaps that’s intentional tho). cheers Iain -- Nathan Sidwell
Re: about souce code location
On 9/4/20 6:26 AM, 易会战 via Gcc wrote: how to check the location corresponding to a gimple statement? My instrument stmt include some memory access, I wish get right source code line. By context it is possible get wrong line. The gimple_location() function returns the location of the GIMPLE statement passed to it as an argument. When the location isn't available (e.g., because the statement corresponds to two or more source lines) there isn't much you can do to retrieve it. Sometimes it's possible to come close by using the location of the next (or previous) GIMPLE statement. Other times the best you can do is point at the function body (either the opening or closing curly). Martin ---Original--- From: "Richard Biener"
A problem with one instruction multiple latencies and pipelines
Hi I'm adding a new machine model. I have a problem when writing the "define_insn_reservation" for instruction scheduling. How to write the "define_insn_reservation" for one instruction that there are different latencies and pipelines according to parameter. For example, the ADD (shifted register) instruction in a64fx InstructionOption LatencyPipeline ADD (shifted register) = 0 1 EX* | EAG* = [1-4] && =LSL 1+1 (EXA + EXA) | (EXB + EXB) 2+1 (EXA + EXA) | (EXB + EXB) In aarch64.md ADD (shifted register) instruction is defined as following. (define_insn "*add__" [(set (match_operand:GPI 0 "register_operand" "=r") (plus:GPI (ASHIFT:GPI (match_operand:GPI 1 "register_operand" "r") (match_operand:QI 2 "aarch64_shift_imm_" "n")) (match_operand:GPI 3 "register_operand" "r")))] "" "add\\t%0, %3, %1, %2" [(set_attr "type" "alu_shift_imm")] ) It could not be distinguished by the type "alu_shift_imm" when writing "define_insn_reservation" for ADD (shifted register). What should I do? Regards Qian