Re: Proposal for merging scalar-storage-order branch into mainline
> Oh and I see a case where we want to remove byteswaps at IPA level. If > we can see the variable value does not escape. That should be relatively easily doable, although I'm a little skeptical of its practical usefulness. -- Eric Botcazou
Re: debug-early branch merged into mainline
On 2015.06.06 at 18:52 -0400, Aldy Hernandez wrote: > On 06/06/2015 05:47 PM, Aldy Hernandez wrote: > > On 06/06/2015 03:33 PM, Jan Hubicka wrote: > >> Aldy, > >> also at PPC64le LTO bootstrap (at gcc112) dies with: > >> ^ > >> 0x104ae8f7 check_die > >> ../../gcc/dwarf2out.c:5715 > > > > Hmmm... this is in the LTO/ltrans stage? If so, that's weird. The LTO > > path does not do the early DIE dance. Since check_die() is a new sanity > > check for DIEs, I wonder if this is a latent bug that was already there. > > It looks like ppc64le fails to bootstrap with the same comparison > failure aarch64 fails with, so I need to take a look at ppc64le regardless. > > However, for your particular problem, I wonder if this was a preexisting > condition. Would you mind reproducing your problem without my > debug-early patchset, but with the attached patch? > > The commit prior to debug-early is: > > git commit d51560f9afc5c8a826bcfa6fc90a96156b623559 > trunk@224160 > > The attached patch adds the sanity check, but without the debug-early > work. If you still get a failure, this is a pre-existing bug. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66468 for a small testcase. It looks like this is not a pre-existing issue, because with your sanity check there is not failure. -- Markus
Re: Proposal for merging scalar-storage-order branch into mainline
> How is this represented in DWARF? This is not represented on the branch, because this cannot be done in pure DWARF. DW_AT_endianity only applies to base types or stand-alone objects and we would need it for DW_TAG_member (and even DW_TAG_array_type in Ada). But this could easily be implemented once the representation is agreed on. -- Eric Botcazou
Re: Proposal for merging scalar-storage-order branch into mainline
On Tue, Jun 09, 2015 at 12:17:49PM +0200, Eric Botcazou wrote: > > How is this represented in DWARF? > > This is not represented on the branch, because this cannot be done in pure > DWARF. DW_AT_endianity only applies to base types or stand-alone objects and > we would need it for DW_TAG_member (and even DW_TAG_array_type in Ada). But > this could easily be implemented once the representation is agreed on. DW_AT_endianity attribute is listed in DWARF4 for: DW_TAG_base_type DW_TAG_constant DW_TAG_formal_parameter DW_TAG_variable Not really sure how to interpret it e.g. on DW_TAG_variable (or formal_parameter) if it has DW_TAG_reference_type type - does it talk about the endianity of what it refers to, or the reference itself, both? Anyway, the DWARF standard doesn't forbid using it on other kinds of DIEs and I think emitting it on DW_TAG_member would be natural. Not sure why you would want it on DW_TAG_array_type, the endianity for arrays should be specified on the element type, shouldn't it? Or is array indexing endian dependent? Jakub
Re: Proposal for merging scalar-storage-order branch into mainline
On Tue, Jun 9, 2015 at 12:33 PM, Jakub Jelinek wrote: > On Tue, Jun 09, 2015 at 12:17:49PM +0200, Eric Botcazou wrote: >> > How is this represented in DWARF? >> >> This is not represented on the branch, because this cannot be done in pure >> DWARF. DW_AT_endianity only applies to base types or stand-alone objects and >> we would need it for DW_TAG_member (and even DW_TAG_array_type in Ada). But >> this could easily be implemented once the representation is agreed on. > > DW_AT_endianity attribute is listed in DWARF4 for: > DW_TAG_base_type > DW_TAG_constant > DW_TAG_formal_parameter > DW_TAG_variable > Not really sure how to interpret it e.g. on DW_TAG_variable (or > formal_parameter) if it has DW_TAG_reference_type type - does it talk > about the endianity of what it refers to, or the reference itself, both? > Anyway, the DWARF standard doesn't forbid using it on other kinds of DIEs > and I think emitting it on DW_TAG_member would be natural. > Not sure why you would want it on DW_TAG_array_type, the endianity for > arrays should be specified on the element type, shouldn't it? > Or is array indexing endian dependent? Maybe endian is index dependent (heh, you never know with Ada!) Richard. > Jakub
Re: Proposal for merging scalar-storage-order branch into mainline
> What's the reason to not expose the byte swapping operations earlier, like > on GIMPLE? (or even on GENERIC?) That would be too heavy, every load and store in GENERIC/GIMPLE would have an associated byte swapping operation, although you don't know if they will be needed in the end. For example, if the structure is scalarized, they are not. > What frontends are affected? The branch contains a working implementation for the C and Ada front-ends, and the beginning of an implementation for the C++ front-end. -- Eric Botcazou
Re: Proposal for merging scalar-storage-order branch into mainline
> Anyway, the DWARF standard doesn't forbid using it on other kinds of DIEs > and I think emitting it on DW_TAG_member would be natural. Agreed. > Not sure why you would want it on DW_TAG_array_type, the endianity for > arrays should be specified on the element type, shouldn't it? For the C family of languages where array types are not first-class citizens, the SSO attribute simply cannot be supported on array types. But for other languages, like Ada, where they are, it can and it is supported on the branch, so you can declare an array of 4 integers with big-endian storage order. -- Eric Botcazou
Re: Proposal for merging scalar-storage-order branch into mainline
On Tue, Jun 9, 2015 at 12:39 PM, Eric Botcazou wrote: >> What's the reason to not expose the byte swapping operations earlier, like >> on GIMPLE? (or even on GENERIC?) > > That would be too heavy, every load and store in GENERIC/GIMPLE would have an > associated byte swapping operation, although you don't know if they will be > needed in the end. For example, if the structure is scalarized, they are not. Yes, but I'd expect them to be optimized away (well, hopefully). Anwyay, and I thought use of the feature would be rare so that "every load and store" is still very few? I suppose doing the lowering somewhen during the GIMPLE optimization pipeline would still be possible to address missed-optimization concerns. >> What frontends are affected? > > The branch contains a working implementation for the C and Ada front-ends, and > the beginning of an implementation for the C++ front-end. Ok, I see. Richard. > -- > Eric Botcazou
Re: Proposal for merging scalar-storage-order branch into mainline
> Yes, but I'd expect them to be optimized away (well, hopefully). OK, but you cannot reasonably expose everything in GENERIC/GIMPLE, for example the mask-and-shift operations to extract bitfields in reverse SSO, only the RTL expander has the (quite complex) logic and I doubt you want to teach the GIMPLE optimizers about this low-level mess. > Anwyay, and I thought use of the feature would be rare so that "every load > and store" is still very few? Experience has shown that, when users start to use it, they use it extensively (too extensively if you ask me) so the implementation cannot be too dumb. -- Eric Botcazou
Re: Proposal for merging scalar-storage-order branch into mainline
On Tue, Jun 9, 2015 at 1:12 PM, Eric Botcazou wrote: >> Yes, but I'd expect them to be optimized away (well, hopefully). > > OK, but you cannot reasonably expose everything in GENERIC/GIMPLE, for example > the mask-and-shift operations to extract bitfields in reverse SSO, only the > RTL expander has the (quite complex) logic and I doubt you want to teach the > GIMPLE optimizers about this low-level mess. Why would you want to support this on bitfields ... (/me runs away). >> Anwyay, and I thought use of the feature would be rare so that "every load >> and store" is still very few? > > Experience has shown that, when users start to use it, they use it extensively > (too extensively if you ask me) so the implementation cannot be too dumb. :/ > -- > Eric Botcazou
Re: Proposal for merging scalar-storage-order branch into mainline
> On Jun 9, 2015, at 7:53 PM, Richard Biener wrote: > > On Tue, Jun 9, 2015 at 1:12 PM, Eric Botcazou wrote: >>> Yes, but I'd expect them to be optimized away (well, hopefully). >> >> OK, but you cannot reasonably expose everything in GENERIC/GIMPLE, for >> example >> the mask-and-shift operations to extract bitfields in reverse SSO, only the >> RTL expander has the (quite complex) logic and I doubt you want to teach the >> GIMPLE optimizers about this low-level mess. > > Why would you want to support this on bitfields ... (/me runs away). Because some folks don't want to audit their code to where to add byteswaps. I am serious people have legacy big-endian code they want to run little endian. There is a reason this is around in the first place. Developers are lazy. Thanks, Andrew > >>> Anwyay, and I thought use of the feature would be rare so that "every load >>> and store" is still very few? >> >> Experience has shown that, when users start to use it, they use it >> extensively >> (too extensively if you ask me) so the implementation cannot be too dumb. > > :/ > >> -- >> Eric Botcazou
Re: [RFC] Kernel livepatching support in GCC
On 06/04/2015 09:15 AM, Ondřej Bílka wrote: > On Thu, May 28, 2015 at 05:37:53PM +0200, Andreas Krebbel wrote: >> On 05/28/2015 11:16 AM, Maxim Kuvyrkov wrote: On May 28, 2015, at 11:59 AM, Richard Biener wrote: >> ... Maybe follow s390 -mhotpatch instead? >>> >>> Regarding implementation of the option, it will follow what s390 is doing >>> with function attributes to mark which functions to apply nop-treatment to >>> (using attributes will avoid problems with [coming] LTO builds of the >>> kernel). The new option will set value of the attribute on all functions >>> in current compilation unit, and then nops will be generated from the >>> attribute specification. >>> >>> On the other hand, s390 does not generate a section of descriptor entries >>> of NOP pads, which seems like a useful (or necessary) option. A >>> more-or-less generic implementation should, therefore, combine s390's >>> attributes approach to annotating functions and x86's approach to providing >>> information in an ELF section about NOP entries. Or can we record value of >>> a function attribute in ELF in a generic way? >> >> I agree that would be useful. The only reason we didn't implement that was >> that our kernel guys were >> confident enough to be able to detect patchable functions without it. We >> discussed two solutions to >> that: >> >> 1. Add special relocations pointing to the patchable areas. >> 2. Add a special section listing all patchable areas. I think systemtap is >> doing something similiar >> for their probes. >> > As I am bit concerned with performance why require nops there? Add a > byte count number >= requested thats boundary of next instruction. When > lifepatching for return you need to copy this followed by jump back to next > instruction. Then gcc could fill that with instructions that don't > depend on address, fill with nops as trivial first implementation. > > Would that be possible? So instead of placing NOPs to be overwritten you intend to simply overwrite the existing code after making a backup of it? While that sounds appealing I think you could run into trouble when trying to atomically patch the code. On S/390 it could happen that you overwrite 3 2 byte instructions with a 6 byte instruction. The effect of the first two might be visible when doing the update so you would have to keep all CPUs outside this area when patching. With the -mhotpatch option it is guaranteed that a 6 byte NOP is used. With the proper alignment we can do an atomic update then. For your approach we have to arrange that bigger instructions get scheduled first and if this isn't possible we would have to add proper NOPs instead. That might be an improvement which could be added to that feature. Btw. we have compared several benchmarks with kernels either using or not using -mhotpatch and there was no noticable penalty. However, I agree that it would be nice to get rid of the NOPs althogether. -Andreas-
Re: [RFC] Kernel livepatching support in GCC
> > As I am bit concerned with performance why require nops there? Add a > > byte count number >= requested thats boundary of next instruction. When > > lifepatching for return you need to copy this followed by jump back to next > > instruction. Then gcc could fill that with instructions that don't > > depend on address, fill with nops as trivial first implementation. > > > > Would that be possible? > > So instead of placing NOPs to be overwritten you intend to simply overwrite > the existing code after > making a backup of it? This is how Linux k/uprobes work. But it only works for a subset of instructions and is fairly complicated because you need a complete decoder that is able to adjust any program counter relative data offsets. Having a patch area is far easier and more reliable. -Andi -- a...@linux.intel.com -- Speaking for myself only
Re: Proposal for merging scalar-storage-order branch into mainline
> Why would you want to support this on bitfields ... (/me runs away). This was the only supported case in the original specification. :-) -- Eric Botcazou
Re: Proposal for merging scalar-storage-order branch into mainline
> Because some folks don't want to audit their code to where to add byteswaps. > I am serious people have legacy big-endian code they want to run little > endian. There is a reason this is around in the first place. Developers are > lazy. That's a little rough, but essentially correct in our experience. -- Eric Botcazou
gcc-5-20150609 is now available
Snapshot gcc-5-20150609 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/5-20150609/ 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 224305 You'll find: gcc-5-20150609.tar.bz2 Complete GCC MD5=f067faa0206b2888e98cf92d1f4580bd SHA1=c779d8f82a2a57425e8484fb2d8ebf2158808b08 Diffs from 5-20150602 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.
Re: consistent naming of passes....
Hi!
Funny that I a) had come across the same issue mentioned below, and then
b) today, while searching for something else in my mail archive, stumbled
over this (unanswered) email. ;-)
On Wed, 27 Aug 2014 20:29:17 +0200, Basile Starynkevitch
wrote:
> When I compile some file (precisely, the gcc/melt-runtime.cc from the latest
> melt branch) with -O1 -fdump-passes (using GCC 4.9) I'm getting
> notably
>
>ipa-cp : OFF
>ipa-cdtor : OFF
>ipa-inline : ON
>ipa-pure-const : ON
>ipa-static-var : ON
>ipa-pta : OFF
>ipa-simdclone : OFF
>*free_cfg_annotations : ON
>
> However, in file gcc/ipa-inline.c there is
>
> const pass_data pass_data_ipa_inline =
> {
> IPA_PASS, /* type */
> "inline", /* name */
> OPTGROUP_INLINE, /* optinfo_flags */
> false, /* has_gate */
> true, /* has_execute */
> TV_IPA_INLINING, /* tv_id */
>
> I find strange that the two names (the one given by -fdump-passes and the one
> in the pass_data_ipa_inline object) are different.
Similarly, for the tags used in filenames for in -fdump-tree-[...] output
-- which "annoys" me, as it requires to do an extra lookup when
correlating the -fdump-tree-[...] filenames, -fdump-passes output,
gcc/passes.def descriptors, and the actual pass definitions.
In the longer term, is it OK to do something about this (but I have not
yet look what needs to be done), to bring in some consistency?
> When I try to insert a plugin pass (actually in MELT, file
> gcc/melt/xtramelt-ana-simple.melt) named "inline" it gives:
>
> cc1plus: fatal error: pass 'inline' not found but is referenced by new pass
> 'melt_justcountipa'
>
> If I use "ipa-inline" I'm getting
> cc1plus: fatal error: pass 'ipa-inline' not found but is referenced by new
> pass 'melt_justcountipa'
>
> How should a plugin writer find the name of the reference pass to insert his
> own new pass? At the very least it should be documented, and preferably it
> should be identical to output of -fdump-passes
Grüße,
Thomas
signature.asc
Description: PGP signature
