Re: RFC: Support non-standard extension (call via casted function pointer)
On January 26, 2016 8:25:50 AM GMT+01:00, Jeff Law wrote: >On 01/26/2016 12:01 AM, Richard Biener wrote: >> On January 25, 2016 10:47:10 PM GMT+01:00, Michael Karcher > wrote: >>> Hello gcc developers, >>> >>> as discussed in https://ghc.haskell.org/trac/ghc/ticket/11395 (and >>> forwarded as PR c/69221), ghc generates non-compliant C code that is >>> not >>> compiled as intended on m68k. This is because its internal Cmm (a >C-- >>> dialect) to C compiler needs to declare external functions (no >varargs) >>> without fixing the type. It currently uses >"unspecified-function-type* >>> function();" which is a quite good fit, because the argument list is >>> still left open, but it fixes the return type to some pointer type. >>> Before calling that function, the function address is put into a >>> function pointer of the correct type and invoked via that pointer. >This >>> model currently works fine (possibly by accident) on all >architectures >>> ghc supports except m68k. >>> >>> I developed a gcc patch that does not change the code generation for >>> conforming programs but fixes this non-conforming use-case by always >>> taking the actual return type in the call expression into account, >even >>> if the function declaration to be called is known. Please comment >>> whether such a patch has any chance to be applied to either gcc >>> upstream >>> or gcc in Debian. >> >> Without looking at the patch this is already how GCC should behave >for all targets. >I believe if they make the return type a pointer type, then the m68k >backend ought to return the value in a0 and d0 to make broken code like > >this work. It may also be the case that we're not doing this properly >for indirect calls from a quick scan of m68k_function_arg. > >The only way to know for sure would be to examine it under the >debugger. What I wanted to say is that we preserve the function type used for the actual call at least until RTL expansion in gimple_call_fntype. I fixed most of the call expansion code to honor that but back ends may of course screw up as they still may somehow get at the actual DECL called. Richard. >Jeff
Re: RFC: Support non-standard extension (call via casted function pointer)
Hi Richard! On 01/26/2016 08:01 AM, Richard Biener wrote: >> I developed a gcc patch that does not change the code generation for >> conforming programs but fixes this non-conforming use-case by always >> taking the actual return type in the call expression into account, even >> if the function declaration to be called is known. Please comment >> whether such a patch has any chance to be applied to either gcc >> upstream >> or gcc in Debian. > > Without looking at the patch this is already how GCC should behave for all > targets. So, would you actually favor the inclusion of Michael's changes? Having gcc allow to work with such code would actually allow us to bootstrap ghc on m68k again which would be awesome :). Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: Question about how to fix PR69052
On Mon, Jan 25, 2016 at 7:51 PM, Jeff Law wrote: > On 01/25/2016 11:42 AM, Bin.Cheng wrote: >> >> >> Yuri Rumyantsev suggested we may add a hook to handle GOT related >> instruction propagation specially so it won't be hoisted out. Is a >> hook at this stage sounds feasible? > > I think that would be a mistake. ISTM this really needs to be cost driven. > >> >> Also it would be great if we can abstract an interface out of combine, >> and the interface can be used in other passes checking whether >> instructions can be merged or not. I know this will be even harder >> than emulation of combine behavior. >> >> Another point is if we can do combine among different basic blocks? > > No, the combiner works within a basic block only. There was a group, I > believe in Moscow, that worked on a cross-block combiner. It was discussed > at the Cauldron in California a few years back. I don't know if they did > any further work on those ideas. > > Is the issue here not knowing when the loop invariant can be forward > propagated back into the memory reference -- and for complex RTL like you > cited, you ultimately determine that no it can't be propagated and thus > increase its invariant cost? Yes, loop invariant now increased invariant cost if the invariant can't be propagated into address expression. Problem is we check propagation by simply replacing use with def in memory reference then verifying result insn with verify_changes. Apparently more advanced transforms are necessary, just like what combine does. I am surprised that rtl_fwprop_addr doesn't handle this either, it's an exact address expression propagation example. Thanks, bin > > jeff
Re: Question about how to fix PR69052
On Mon, Jan 25, 2016 at 8:05 PM, Bernd Schmidt wrote: > On 01/25/2016 08:51 PM, Jeff Law wrote: >> >> No, the combiner works within a basic block only. There was a group, I >> believe in Moscow, that worked on a cross-block combiner. It was >> discussed at the Cauldron in California a few years back. I don't know >> if they did any further work on those ideas. >> >> Is the issue here not knowing when the loop invariant can be forward >> propagated back into the memory reference -- and for complex RTL like >> you cited, you ultimately determine that no it can't be propagated and >> thus increase its invariant cost? > > > Is this just a pass ordering problem? What happens if you do loop-invariant > after combine? Yes, I moved whole loop pass (also the pass_web) after combine and it worked. A combine pass before loop-invariant can fix this problem. Below passes are currently between loop transform and combine: NEXT_PASS (pass_web); NEXT_PASS (pass_rtl_cprop); NEXT_PASS (pass_cse2); NEXT_PASS (pass_rtl_dse1); NEXT_PASS (pass_rtl_fwprop_addr); NEXT_PASS (pass_inc_dec); NEXT_PASS (pass_initialize_regs); NEXT_PASS (pass_ud_rtl_dce); I think pass_web needs to be after loop transform because it's used to handle unrolled register live range. pass_fwprop_addr and pass_inc_dec should stay where they are now. And putting pass_inc_dec before loop unroll may be helpful to keep more auto increment addressing mode chosen by IVO. We should not need to duplicate pass_initialize_regs. So what's about pass_rtl_cprop, cse2 and dse1. Should these pass be duplicated after loop transform thus loop transformed code can be cleaned up? Thanks, bin > > > Bernd >
Re: RFC: Support non-standard extension (call via casted function pointer)
John Paul Adrian Glaubitz writes: > Having gcc allow to work with such code would actually allow us > to bootstrap ghc on m68k again which would be awesome :). The ghc code just needs to be fixed to not lie in such a blatant way. Just like it was changed when ppc64le flagged this as crap code. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: RFC: Support non-standard extension (call via casted function pointer)
On 01/26/2016 11:07 AM, Andreas Schwab wrote: > John Paul Adrian Glaubitz writes: > >> Having gcc allow to work with such code would actually allow us >> to bootstrap ghc on m68k again which would be awesome :). > > The ghc code just needs to be fixed to not lie in such a blatant way. > Just like it was changed when ppc64le flagged this as crap code. I could just find one bug report which mentions a fix for ppc64el [1], are you talking about this one? If this bug is actually the same as the m68k bug, why is ghc still working fine on ppc64el? Does ppc64el actually have separate address and data registers? Adrian > [1] https://ghc.haskell.org/trac/ghc/ticket/8965 -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: RFC: Support non-standard extension (call via casted function pointer)
John Paul Adrian Glaubitz writes: > I could just find one bug report which mentions a fix for ppc64el [1], > are you talking about this one? Yes. > If this bug is actually the same as the m68k bug, Where did I say that? > why is ghc still working fine on ppc64el? Because ghc tweaked its crap code just enough for ppc64le. But it can break any time again, because it is still crap code. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: RFC: Support non-standard extension (call via casted function pointer)
On 01/26/2016 12:07 PM, Andreas Schwab wrote: >> If this bug is actually the same as the m68k bug, > > Where did I say that? > The ghc code just needs to be fixed to not lie in such a blatant way. > Just like it was changed when ppc64le flagged this as crap code. ^ But I don't want to argue anyway. I was assuming ppc64le might have similarities to m68k in that regard. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: [RFC] DW_OP_piece vs. DW_OP_bit_piece on a Register
On Mon, Jan 25 2016, Matthew Fortune wrote: > My dwarf knowledge is not brilliant but I have had to recently consider > it for MIPS floating point ABI changes aka FPXX and friends. I don't know > exactly where this fits in to your whole description but in case it has > a bearing on this we now have the following uses of DW_OP_piece: > > 1) double precision data split over two 32-bit FPRs > Uses a pair of 32-bit DW_OP_piece (ordered depending on endianness). > > 2) double precision data in one 64-bit FPR > No need for DW_OP_piece. > > 3) double precision data that may be in two 32-bit FPRs or may be in >one 64-bit FPR depending on hardware mode > Uses a single 64-bit DW_OP_piece on the even numbered register. Hm, so in 32-bit hardware mode the DWARF consumer is expected to treat the DW_OP_piece on the even numbered register as if it were two pieces from two consecutive registers? Or should we rather consider the even numbered register to be 64 bit wide, where the right half shadows the next odd-numbered register? If so, I believe you generally want pieces from FPRs to be taken from the left, correct? > I'm guilty of not actually finishing this off and doing the GDB side but > the theory seemed OK when I did it! From your description this behaviour > best matches DW_OP_piece having ABI dependent behaviour which would make > it acceptable. These three variations can potentially exist in the same > program albeit that (1) and (3) can't appear in a single shared library > or executable. It's all a little strange but the whole floating point > MIPS o32 ABI is pretty complex now. I don't quite understand why (1) and (3) can not coexist in the same shared library or executable. Can you elaborate a bit? I'm curious about the interaction with vector registers. AFAIK, vector registers on MIPS also embed the FPRs (left or right?). Are the same DWARF register numbers used for both? And when taking a 64-bit DW_OP_piece from a vector register, would this piece correspond to the embedded FPR? Also, how do you think that the following should be represented in DWARF: * Odd-sized bit field in one of a vector register's elements; * odd-sized bit field spilled into an FPR; * single-precision `complex float' living in two consecutive 64-bit FPRs. Thanks for your input! -- Andreas
Re: Question about how to fix PR69052
On 01/26/2016 10:48 AM, Bin.Cheng wrote: Yes, I moved whole loop pass (also the pass_web) after combine and it worked. A combine pass before loop-invariant can fix this problem. Below passes are currently between loop transform and combine: NEXT_PASS (pass_web); NEXT_PASS (pass_rtl_cprop); NEXT_PASS (pass_cse2); NEXT_PASS (pass_rtl_dse1); NEXT_PASS (pass_rtl_fwprop_addr); NEXT_PASS (pass_inc_dec); NEXT_PASS (pass_initialize_regs); NEXT_PASS (pass_ud_rtl_dce); I think pass_web needs to be after loop transform because it's used to handle unrolled register live range. pass_fwprop_addr and pass_inc_dec should stay where they are now. And putting pass_inc_dec before loop unroll may be helpful to keep more auto increment addressing mode chosen by IVO. We should not need to duplicate pass_initialize_regs. So what's about pass_rtl_cprop, cse2 and dse1. Should these pass be duplicated after loop transform thus loop transformed code can be cleaned up? Hard to tell in advance - you might want to experiment a bit; set up a large collection of input files and see what various approaches do to code generation. In any case, I suspect this is gcc-7 material (unfortunately). Bernd
Re: Question about how to fix PR69052
On Tue, Jan 26, 2016 at 12:56 PM, Bernd Schmidt wrote: > On 01/26/2016 10:48 AM, Bin.Cheng wrote: >> >> Yes, I moved whole loop pass (also the pass_web) after combine and it >> worked. A combine pass before loop-invariant can fix this problem. >> Below passes are currently between loop transform and combine: >> >>NEXT_PASS (pass_web); >>NEXT_PASS (pass_rtl_cprop); >>NEXT_PASS (pass_cse2); >>NEXT_PASS (pass_rtl_dse1); >>NEXT_PASS (pass_rtl_fwprop_addr); >>NEXT_PASS (pass_inc_dec); >>NEXT_PASS (pass_initialize_regs); >>NEXT_PASS (pass_ud_rtl_dce); >> >> I think pass_web needs to be after loop transform because it's used to >> handle unrolled register live range. >> pass_fwprop_addr and pass_inc_dec should stay where they are now. And >> putting pass_inc_dec before loop unroll may be helpful to keep more >> auto increment addressing mode chosen by IVO. >> We should not need to duplicate pass_initialize_regs. >> So what's about pass_rtl_cprop, cse2 and dse1. Should these pass be >> duplicated after loop transform thus loop transformed code can be >> cleaned up? > > > Hard to tell in advance - you might want to experiment a bit; set up a large > collection of input files and see what various approaches do to code > generation. In any case, I suspect this is gcc-7 material (unfortunately). Quite surprising, there is only one new failure after moving loop transforms (only along with pass_web). Yes, it is GCC7 change, will revisit this to see if it makes substantial code generation difference. Anyway, for the bug itself, there might be simple fix in x86 backend according to newest update. Thanks, bin
Re: Question about how to fix PR69052
On Tue, Jan 26, 2016 at 1:51 PM, Bin.Cheng wrote: > On Tue, Jan 26, 2016 at 12:56 PM, Bernd Schmidt > wrote: >> On 01/26/2016 10:48 AM, Bin.Cheng wrote: >>> >>> Yes, I moved whole loop pass (also the pass_web) after combine and it >>> worked. A combine pass before loop-invariant can fix this problem. >>> Below passes are currently between loop transform and combine: >>> >>>NEXT_PASS (pass_web); >>>NEXT_PASS (pass_rtl_cprop); >>>NEXT_PASS (pass_cse2); >>>NEXT_PASS (pass_rtl_dse1); >>>NEXT_PASS (pass_rtl_fwprop_addr); >>>NEXT_PASS (pass_inc_dec); >>>NEXT_PASS (pass_initialize_regs); >>>NEXT_PASS (pass_ud_rtl_dce); >>> >>> I think pass_web needs to be after loop transform because it's used to >>> handle unrolled register live range. >>> pass_fwprop_addr and pass_inc_dec should stay where they are now. And >>> putting pass_inc_dec before loop unroll may be helpful to keep more >>> auto increment addressing mode chosen by IVO. >>> We should not need to duplicate pass_initialize_regs. >>> So what's about pass_rtl_cprop, cse2 and dse1. Should these pass be >>> duplicated after loop transform thus loop transformed code can be >>> cleaned up? >> >> >> Hard to tell in advance - you might want to experiment a bit; set up a large >> collection of input files and see what various approaches do to code >> generation. In any case, I suspect this is gcc-7 material (unfortunately). > > Quite surprising, there is only one new failure after moving loop > transforms (only along with pass_web). Yes, it is GCC7 change, will For the record. The only new failure is loop-9.c. Given insn patterns as below: Loop: 23: r106:DF=[`*.LC0'] REG_EQUAL 1.842419990222931955941021442413330078125e+1 24: [r101:DI]=r106:DF insn 23 is merged into insn 24 by combine if it's run before loop_invariant, resulting in below insn: 23: NOTE_INSN_DELETED 24: [r101:DI]=1.842419990222931955941021442413330078125e+1 But we can't support the floating constant, so insn24 will be split anyway by reload: 35: xmm0:DF=[`*.LC0'] 24: [di:DI]=xmm0:DF This time both instructions are in loop, causing a regression. Combine may need some change if we run it before loop_invariant. Thanks, bin > revisit this to see if it makes substantial code generation > difference. Anyway, for the bug itself, there might be simple fix in > x86 backend according to newest update. > > Thanks, > bin
Re: RFC: Support non-standard extension (call via casted function pointer)
On Mon, Jan 25, 2016 at 10:47:10PM +0100, Michael Karcher wrote: > Hello gcc developers, > > as discussed in https://ghc.haskell.org/trac/ghc/ticket/11395 (and > forwarded as PR c/69221), ghc generates non-compliant C code that is not > compiled as intended on m68k. This is because its internal Cmm (a C-- > dialect) to C compiler needs to declare external functions (no varargs) > without fixing the type. It currently uses "unspecified-function-type* > function();" which is a quite good fit, because the argument list is > still left open, but it fixes the return type to some pointer type. > Before calling that function, the function address is put into a > function pointer of the correct type and invoked via that pointer. This > model currently works fine (possibly by accident) on all architectures > ghc supports except m68k. we have seen this problem come up plenty of times, so far it was always solved by repairing the offending program. >From experience I would say that whatever ghc is trying to do can be done correctly and ghc should try that. Richard -- Name and OpenPGP keys available from pgp key servers
Re: RFC: Support non-standard extension (call via casted function pointer)
On 01/26/2016 01:41 AM, Richard Biener wrote: What I wanted to say is that we preserve the function type used for the actual call at least until RTL expansion in gimple_call_fntype. I fixed most of the call expansion code to honor that but back ends may of course screw up as they still may somehow get at the actual DECL called. I think that's a good thing -- however, I suspect there's m68k backend issues that need to be addressed here. So I think we're roughly on the same page. jeff
Re: RFC: Support non-standard extension (call via casted function pointer)
On 01/26/2016 02:21 AM, John Paul Adrian Glaubitz wrote: Hi Richard! On 01/26/2016 08:01 AM, Richard Biener wrote: I developed a gcc patch that does not change the code generation for conforming programs but fixes this non-conforming use-case by always taking the actual return type in the call expression into account, even if the function declaration to be called is known. Please comment whether such a patch has any chance to be applied to either gcc upstream or gcc in Debian. Without looking at the patch this is already how GCC should behave for all targets. So, would you actually favor the inclusion of Michael's changes? Having gcc allow to work with such code would actually allow us to bootstrap ghc on m68k again which would be awesome :). Not as-is. But they make a starting point for trying to address the problem -- particularly the testcases, even if they are ill-formed to a certain degree. jeff
Re: RFC: Support non-standard extension (call via casted function pointer)
On 01/26/2016 03:59 AM, John Paul Adrian Glaubitz wrote: On 01/26/2016 11:07 AM, Andreas Schwab wrote: John Paul Adrian Glaubitz writes: Having gcc allow to work with such code would actually allow us to bootstrap ghc on m68k again which would be awesome :). The ghc code just needs to be fixed to not lie in such a blatant way. Just like it was changed when ppc64le flagged this as crap code. I could just find one bug report which mentions a fix for ppc64el [1], are you talking about this one? If this bug is actually the same as the m68k bug, why is ghc still working fine on ppc64el? Does ppc64el actually have separate address and data registers? Ignore other targets. There's nothing really shared across them when it comes to the low level implementation details of an ABI like this. jeff
Re: RFC: Support non-standard extension (call via casted function pointer)
On Tue, Jan 26, 2016 at 10:21 AM, John Paul Adrian Glaubitz wrote: > Hi Richard! > > On 01/26/2016 08:01 AM, Richard Biener wrote: >>> I developed a gcc patch that does not change the code generation for >>> conforming programs but fixes this non-conforming use-case by always >>> taking the actual return type in the call expression into account, even >>> if the function declaration to be called is known. Please comment >>> whether such a patch has any chance to be applied to either gcc >>> upstream >>> or gcc in Debian. >> >> Without looking at the patch this is already how GCC should behave for all >> targets. > > So, would you actually favor the inclusion of Michael's changes? No, the patch looks somewhat broken to me. A complete fix would replace the target macro FUNCTION_VALUE implementation by implementing the function_value hook instead (to also receive the function type called if no decl is available and to be able to distinguish caller vs. callee). I also don't see how changing the called function code will fix anything. In fact the frobbing of the return value into %d0 should only happen if the 'outgoing' arg is true (in the hook implementation) and in the 'outgoing' == false case simply disregard 'func' and always use 'valtype'. So, hookize and change to if (outgoing && POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (func ... else if (POINTER_TYPE_P (valtype)) ... else ... Richard. > Having gcc allow to work with such code would actually allow us > to bootstrap ghc on m68k again which would be awesome :). > > Adrian > > -- > .''`. John Paul Adrian Glaubitz > : :' : Debian Developer - glaub...@debian.org > `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de > `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: Question about how to fix PR69052
On 01/26/2016 02:28 AM, Bin.Cheng wrote: Yes, loop invariant now increased invariant cost if the invariant can't be propagated into address expression. Problem is we check propagation by simply replacing use with def in memory reference then verifying result insn with verify_changes. Apparently more advanced transforms are necessary, just like what combine does. I am surprised that rtl_fwprop_addr doesn't handle this either, it's an exact address expression propagation example. So the question now becomes whether or not the work done by combine.c is dependent on knowledge that is specific to combine or is it just a series of transformations that we could make available (preferably moving it to simplify-rtx). If we can factor the transformation out and shove it into simplify-rtx, then it could be used elsewhere. jeff
Re: Question about how to fix PR69052
On Tue, Jan 26, 2016 at 4:26 PM, Jeff Law wrote: > On 01/26/2016 02:28 AM, Bin.Cheng wrote: >> >> Yes, loop invariant now increased invariant cost if the invariant >> can't be propagated into address expression. Problem is we check >> propagation by simply replacing use with def in memory reference then >> verifying result insn with verify_changes. Apparently more advanced >> transforms are necessary, just like what combine does. >> I am surprised that rtl_fwprop_addr doesn't handle this either, it's >> an exact address expression propagation example. > > So the question now becomes whether or not the work done by combine.c is > dependent on knowledge that is specific to combine or is it just a series of > transformations that we could make available (preferably moving it to > simplify-rtx). > > If we can factor the transformation out and shove it into simplify-rtx, then > it could be used elsewhere. According to comment at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69052#c6, it should be just re-arrangement of operands, if the comment itself holds. In this way, the transformation is backend related, maybe better to keep it in backend, right? Thanks, bin > > jeff
Re: Question about how to fix PR69052
On 01/26/2016 09:36 AM, Bin.Cheng wrote: On Tue, Jan 26, 2016 at 4:26 PM, Jeff Law wrote: On 01/26/2016 02:28 AM, Bin.Cheng wrote: Yes, loop invariant now increased invariant cost if the invariant can't be propagated into address expression. Problem is we check propagation by simply replacing use with def in memory reference then verifying result insn with verify_changes. Apparently more advanced transforms are necessary, just like what combine does. I am surprised that rtl_fwprop_addr doesn't handle this either, it's an exact address expression propagation example. So the question now becomes whether or not the work done by combine.c is dependent on knowledge that is specific to combine or is it just a series of transformations that we could make available (preferably moving it to simplify-rtx). If we can factor the transformation out and shove it into simplify-rtx, then it could be used elsewhere. According to comment at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69052#c6, it should be just re-arrangement of operands, if the comment itself holds. In this way, the transformation is backend related, maybe better to keep it in backend, right? I'd like to see the key RTL heading into ix86_decompose_address -- mostly to make sure it's not being passed non-canonical RTL or somesuch. jeff
Re: Need forms for contribution towards gcc
On 01/21/2016 11:04 AM, Shiv Shankar Dayal wrote: Hi, I am a C/C++ programmer with 10 years of professional programming experience. I have been using gcc since version 2. I am not a compiler expert but would like to start contributing to gcc for which I need forms which require submission from gcc side. Please send me those and I will fill and send them back. Contact ass...@gnu.org to get the forms you need. Most of the time I would recommend getting past-and-future contributions as well as an employer disclaimer. jeff
Re: RFC: Support non-standard extension (call via casted function pointer)
On 26.01.2016 16:40, Richard Biener wrote: > No, the patch looks somewhat broken to me. A complete fix would replace > the target macro FUNCTION_VALUE implementation by implementing the > function_value hook instead (to also receive the function type called if no > decl is available and to be able to distinguish caller vs. callee). > > I also don't see how changing the called function code will fix anything. I definitely agree that the approach to move from the FUNCTION_VALUE macro to the function_value hook is the most clean way to do it. If there is a consensus among the gcc developers that a patch to support this non-conforming code should be applied, I would be willing to prepare a patch as suggested. The current patch does not change the called code at all. The only time at which my suggested patch changes gcc behaviour is if a function declaration is given, that declaration has a pointer type as return type, but valtype is no pointer type. This (unverified claim) can not happen in the callee, as the valtype when compiling the return statement or assembling the epilogue is taken from the function declaration. > In fact the frobbing of the return value into %d0 should only happen > if the 'outgoing' arg is true (in the hook implementation) and in the > 'outgoing' == false case simply disregard 'func' and always use > 'valtype'. This frobbing of a pointer return value in %d0 only happens in the outgoing case already now, because in the non-outgoing case, m68k_function_value is (unverified claim) only called from expand_call, which replaces the PARALLEL rtx by the first list element, i.e. %a0. (or %d0 if !POINTER_TYPE_P(valtype) after my patch). > So, hookize and change to > > if (outgoing && POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (func > ... > else if (POINTER_TYPE_P (valtype)) >... > else >... Looks good and clean to me, but I expect my patch to have the same effect. Regards, Michael Karcher
Re: Status of GCC 6 on x86_64 (Debian)
On Fri, 2016-01-22 at 08:27 +0100, Matthias Klose wrote:
> On 22.01.2016 06:09, Martin Michlmayr wrote:
> > In terms of build failures, I reported 520 bugs to Debian. Most of them
> > were new GCC errors or warnings (some packages use -Werror and many
> > -Werror=format-security).
> >
> > Here are some of the most frequent errors see:
>
> [...]
> Martin tagged these issues; https://wiki.debian.org/GCC6 has links with these
> bug searches.
Thanks.
The -Wmisleading-indentation ones can be seen at:
https://bugs.debian.org/cgi-bin/pkgreport.cgi?tag=gcc-6-misleading-indentation;users=debian-...@lists.debian.org
I looked through them all.
I opened PR c/69415 to cover some possible issues with how we deal with
one-liners, which a couple of these relate to; I don't know if we want
to tweak our policy for one-liners.
Other than that, I felt that we're in good shape here: I think the
things we warned about were reasonable to warn about (but I have an
obvious bias here). I'm writing up some notes for the website's
"porting to gcc 6" page based on this.
Detailed notes follow:
##
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811572
Package: pimd/litite:
Looking at,
https://github.com/troglobit/libite/blob/master/lite.h#L134
the code in question seems to be:
static inline int fisslashdir(char *dir)
{
if (!dir) return 0;
if (strlen (dir) > 0) return dir[strlen (dir) - 1] == '/';
return 0;
}
##
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811573
Package: rustc
> /<>/rustc-1.5.0+dfsg1/src/rt/miniz.c: In function
> 'tinfl_decompress':
> /<>/rustc-1.5.0+dfsg1/src/rt/miniz.c:578:47: error: statement is
> indented as if it were guarded by... [-Werror=misleading-indentation]
> for ( i = 0; i <= 143; ++i) *p++ = 8; for ( ; i <= 255; ++i) *p++ =
> 9; for ( ; i <= 279; ++i) *p++ = 7; for ( ; i <= 287; ++i) *p++ = 8;
>^~~
Link to upstream rustc code in context:
https://github.com/rust-lang/rust/blob/master/src/rt/miniz.c#L578
https://github.com/rust-lang/rust/blob/master/src/rt/miniz.c#L1396
My opinion: it's fair to issue a warning for this at -Wall
##
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811575
Package: nss
The code in question seems to be here:
https://hg.mozilla.org/projects/nss/file/tip/lib/dbm/src/h_page.c#l117
and contains mixed spaces and tabs. Detabifying at 8-spaces per tab (though
am not sure if this is what upstream nss use), I get:
if(origin == SEEK_CUR)
{
if(offset < 1)
return(lseek(fd, offset, SEEK_CUR));
cur_pos = lseek(fd, 0, SEEK_CUR);
if(cur_pos < 0)
return(cur_pos);
}
IMHO the indentation here *is* misleading and hence the warning is justified.
##
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811576
Package: tpm-tools
Code in question seems to be here:
http://sources.debian.net/src/tpm-tools/1.3.8-2/src/tpm_mgmt/tpm_present.c/#L358
at the end of the function:
out:
if (szTpmPasswd && !isWellKnown)
shredPasswd( szTpmPasswd );
return iRc;
IMHO indentation here is somewhat misleading; warning feels justified
##
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811577
Package: sgt-puzzles
Code in question seems to be here:
https://github.com/radekp/sgt-puzzles/blob/master/towers.c#L391
if (ret)
return ret;
#ifdef STANDALONE_SOLVER
if (solver_show_working)
sprintf(prefix, "%*slower bounds for clue %s %d:\n",
solver_recurse_depth*4, "",
cluepos[c/w], c%w+1);
else
prefix[0] = '\0'; /* placate optimiser */
#endif
Not sure how "misleading" this is, more just poorly indented.
##
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811578
Package: confetti
Looks like it might be generated code
e.g. generated by https://github.com/mailru/confetti/blob/master/c_dump.c
##
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811579
Package: mosh
> make[5]: Entering directory '/<>/src/protobufs'
...
> userinput.pb.cc: In member function 'virtual bool
> ClientBuffers::Instruction::IsInitialized() const':
> userinput.pb.cc:375:53: error: statement is indented as if it were guarded
> by... [-Werror=misleading-indentation]
>if (!_extensions_.IsInitialized()) return false; return true;
>
Re: RFC: Support non-standard extension (call via casted function pointer)
On January 26, 2016 8:03:35 PM GMT+01:00, Michael Karcher wrote: >On 26.01.2016 16:40, Richard Biener wrote: >> No, the patch looks somewhat broken to me. A complete fix would >replace >> the target macro FUNCTION_VALUE implementation by implementing the >> function_value hook instead (to also receive the function type called >if no >> decl is available and to be able to distinguish caller vs. callee). >> >> I also don't see how changing the called function code will fix >anything. >I definitely agree that the approach to move from the FUNCTION_VALUE >macro to the function_value hook is the most clean way to do it. If >there is a consensus among the gcc developers that a patch to support >this non-conforming code should be applied, I would be willing to >prepare a patch as suggested. > >The current patch does not change the called code at all. The only time >at which my suggested patch changes gcc behaviour is if a function >declaration is given, that declaration has a pointer type as return >type, but valtype is no pointer type. This (unverified claim) can not >happen in the callee, as the valtype when compiling the return >statement >or assembling the epilogue is taken from the function declaration. > >> In fact the frobbing of the return value into %d0 should only happen >> if the 'outgoing' arg is true (in the hook implementation) and in the >> 'outgoing' == false case simply disregard 'func' and always use >> 'valtype'. >This frobbing of a pointer return value in %d0 only happens in the >outgoing case already now, because in the non-outgoing case, >m68k_function_value is (unverified claim) only called from expand_call, >which replaces the PARALLEL rtx by the first list element, i.e. %a0. >(or >%d0 if !POINTER_TYPE_P(valtype) after my patch). > >> So, hookize and change to >> >> if (outgoing && POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (func >> ... >> else if (POINTER_TYPE_P (valtype)) >>... >> else >>... >Looks good and clean to me, but I expect my patch to have the same >effect. I would still prefer the more obvious approach of using the target hook transition. Richard. > >Regards, > Michael Karcher
Re: RFC: Support non-standard extension (call via casted function pointer)
On 26.01.2016 21:47, Richard Biener wrote: >>> So, hookize and change to >>> >>> if (outgoing && POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (func >>> ... >>> else if (POINTER_TYPE_P (valtype)) >>>... >>> else >>>... >> Looks good and clean to me, but I expect my patch to have the same >> effect. > I would still prefer the more obvious approach of using the target hook > transition. I intended to express in my last email: Me too, and I will prepare a patch with a target hook if there is consensus that this patch can be accepted. I do not want the current patch committed as-is, I just posted it to get the idea across and start discussion. Regards, Michael Karcher
gcc-5-20160126 is now available
Snapshot gcc-5-20160126 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/5-20160126/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 5 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-5-branch revision 232854 You'll find: gcc-5-20160126.tar.bz2 Complete GCC MD5=67956099bd8ec3d7850b89881904019d SHA1=5320d40630b26bca636445e4ad3430d780ac07b4 Diffs from 5-20160119 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-5 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.
"cc" clobber
It is well known that on i386, the "cc" clobber is always set for
extended asm, whether it is specified or not. I was wondering how much
difference it might make if the generated code actually followed what
the user specified (expectation: not much). But implementing this
turned up a different question.
I started by just commenting out the code in ix86_md_asm_adjust that
unconditionally clobbered the flags. I figured this would allow the
'normal' "cc" handling to occur. But apparently there is no 'normal'
"cc" handling.
So I went back to ix86_md_asm_adjust and tried to handle the "cc" if it
was specified in the clobbers argument. But apparently "cc" doesn't get
added to that clobbers list.
Hmm.
Tracing back to see how the "memory" clobber (which does get added to
the clobber list) is handled brings me to expand_asm_stmt() in
cfgexpand.c. Following the example set by the memory clobber, it looks
like I want something like this:
else if (j == -3)
{
#if defined(__i386__) || defined(__x86_64__)
rtx x = gen_rtx_REG (CCmode, FLAGS_REG);
clobber_rvec.safe_push (x);
x = gen_rtx_REG (CCFPmode, FPSR_REG);
clobber_rvec.safe_push (x);
#endif
}
Now I can check for this in the clobbers to ix86_md_asm_adjust and
SET_HARD_REG_BIT as appropriate. Tada.
It's working, but can that be right? Why do I need to do this for
i386? How do other platforms handle "cc"?
Other than not rejecting it as an invalid clobber, I can't find any code
that seems to recognize "cc." Has "cc" become just an unenforced
comment on all platforms? Or did I just miss it?
dw
Re: "cc" clobber
On 01/27/2016 12:12 AM, David Wohlferd wrote: I started by just commenting out the code in ix86_md_asm_adjust that unconditionally clobbered the flags. I figured this would allow the 'normal' "cc" handling to occur. But apparently there is no 'normal' "cc" handling. I have a dim memory that there's a difference between the "cc" and "CC" spellings. You might want to check that. Bernd
