Re: IPA / Cgraph how to get a real COPY of a function body
Hi, On Tue, Sep 16, 2008 at 12:24:13PM +0200, Martin Schindewolf wrote: > Dear all, > > my current efforts to get some basic support for transactional memory in > GCC advance slowly but there is one thing I would like to ask because it > stopped me for days now. What is the best way to do the function cloning in > order to have the original function and a transactional clone at the same > time? I tried all sorts of cloning and copying the whole tree using > functions from Cgraph and Ipa passes but somehow I always end up with a > cloned declaration node and a function body that is not a copy of the > original but a pointer to it. At least that is my experience until now. > Following the general idea of having a copy of the original function early > in the passes and inserting the necessary instrumentations later when the > expansion of the GTM primitives is done, this behaviour leads to problems. > What would be the best way to get a real copy of a function body? > I believe cgraph_function_versioning() is what you are looking for. Have a look at how it is used in ipa-cp.c. HTH Martin
Re: Changed resolution of anonymous namespace extern from gcc 3.4.2 to 4.1.0
Earl Chew wrote:
I can't make up my mind whether the new behaviour is incorrect,
or whether the old behaviour should never have been supported.
I'm pretty certain this is a defect because I can construct a
program that should work, but doesn't.
In any case, I've discovered that this was an aberration of
4.1.0, and does not appear in later versions.
Consider foo.cpp:
namespace { extern const char f[]; }
namespace { const char f[] = "abc"; }
const char* g = f;
Expect variable g to be global, but f to have non-global scope.
3.4.2 Pass
4.1.0 Fail
4.1.1 Pass
4.1.2 Fail
4.1.2 ***
PASS ***
.file "foo.cpp"
.globl g
.data
.align 8
.type g, @object
.size g, 8
g:
.quad _ZN36_GLOBAL__N_foo.cpp__2B2B2FAE1fE
.section.rodata
.type _ZN36_GLOBAL__N_foo.cpp__2B2B2FAE1fE, @object
.size _ZN36_GLOBAL__N_foo.cpp__2B2B2FAE1fE, 4
_ZN36_GLOBAL__N_foo.cpp__2B2B2FAE1fE:
.string "abc"
.ident "GCC: (GNU) 4.1.2 20070626 (Red Hat 4.1.2-14)"
.section.note.GNU-stack,"",@progbits
4.1.1 ***
PASS ***
.file "foo.cpp"
.globl g
.data
.align 4
.type g, @object
.size g, 4
g:
.long _ZN36_GLOBAL__N_foo.cpp__8E53D9441fE
.section.rodata
.type _ZN36_GLOBAL__N_foo.cpp__8E53D9441fE, @object
.size _ZN36_GLOBAL__N_foo.cpp__8E53D9441fE, 4
_ZN36_GLOBAL__N_foo.cpp__8E53D9441fE:
.string "abc"
.ident "GCC: (GNU) 4.1.1 20070105 (Red Hat 4.1.1-51)"
.section.note.GNU-stack,"",@progbits
4.1.0 ***
FAIL ***
.file "foo.cpp"
.globl _ZN36_GLOBAL__N_foo.cpp__54D6CB4D1fE
.section.rodata
.type _ZN36_GLOBAL__N_foo.cpp__54D6CB4D1fE, @object
.size _ZN36_GLOBAL__N_foo.cpp__54D6CB4D1fE, 4
_ZN36_GLOBAL__N_foo.cpp__54D6CB4D1fE:
.string "abc"
.globl g
.data
.align 4
.type g, @object
.size g, 4
g:
.long _ZN36_GLOBAL__N_foo.cpp__54D6CB4D1fE
.ident "GCC: (GNU) 4.1.0"
.section.note.GNU-stack,"",@progbits
3.4.2 ***
PASS ***
.file "foo.cpp"
.globl _ZN36_GLOBAL__N_foo_cpp__4C2300AC1fE
.section.rodata
.align 2
.type _ZN36_GLOBAL__N_foo_cpp__4C2300AC1fE, @object
.size _ZN36_GLOBAL__N_foo_cpp__4C2300AC1fE, 4
_ZN36_GLOBAL__N_foo_cpp__4C2300AC1fE:
.string "abc"
.globl g
.section".data"
.align 2
.type g, @object
.size g, 4
g:
.long _ZN36_GLOBAL__N_foo_cpp__4C2300AC1fE
.ident "GCC: (GNU) 3.4.2 (mingw-special)"
Dubious dse.c code
While looking at the Darwin (const (minus ...)) thing[*],
I came across some dubious-looking code in dse.c:
/* The groups are different, if the alias sets
conflict, clear the entire group. We only need
to apply this test if the read_info is a cselib
read. Anything with a constant base cannot alias
something else with a different constant
base. */
if ((read_info->group_id < 0)
&& canon_true_dependence (group->base_mem,
QImode,
group->canon_base_mem,
read_info->mem, rtx_varies_p))
{
if (kill)
bitmap_ior_into (kill, group->group_kill);
bitmap_and_compl_into (gen, group->group_kill);
}
The thing to note is that:
- the first argument to canon_true_dependence is supposed to
be the canonicalised MEM and
- the third argument is supposed to be the canonicalised address
All other calls in dse.c seem to be correct. But in this one,
we pass the uncanoncalised MEM and canonicalised MEM respectively.
Thus the first argument isn't canonicalised when we expect it to be,
and the all-important address argument has the wrong value.
Luckily, passing a MEM address usually causes canon_true_dependence
to return true, so we're generally safe.
I would fix this myself, but I don't understand why the code is
algporithmically safe. We create base_mem as follows:
gi->base_mem = gen_rtx_MEM (QImode, base);
...
gi->canon_base_mem = canon_rtx (gi->base_mem);
I.e. the MEM is always a single byte, regardless of the real access size.
Moreover, group_info generally describes a _range_ of BASE+OFFSET MEMs,
not a single MEM. There's no guarantee that BASE+0 is ever actually used.
So how can a true dependence check on (mem:QI BASE) be enough?
Also, this MEM has alias set 0, which seems to contradict the first
part of the comment.
For avoidance of doubt, base_mem and canon_base_mem only exist for
this canon_true_dependence call.
Richard
[*] Which is going well btw. Hope to submit by this weekend.
Re: New assert in haifa-sched.c
> Yes, the assert is really checking exactly that. Several pieces of > haifa-sched.c assume that the instruction has been recognized during > scheduler initialization to speed up checking if instruction is normal > or some kind of use/clobber/asm. What happens if an instruction hasn't been recognized upon reaching these parts of haifa-sched.c? Will it be only mis-scheduled, i.e. will this only result in inferior, but still correct code? If so, the assertion shouldn't be enabled in release mode but only if ENABLE_CHECKING is defined. -- Eric Botcazou
Re: New assert in haifa-sched.c
Eric Botcazou wrote: Yes, the assert is really checking exactly that. Several pieces of haifa-sched.c assume that the instruction has been recognized during scheduler initialization to speed up checking if instruction is normal or some kind of use/clobber/asm. What happens if an instruction hasn't been recognized upon reaching these parts of haifa-sched.c? Will it be only mis-scheduled, i.e. will this only result in inferior, but still correct code? The code will still be correct, but I think there's more than 1 assert relying on this property, so GCC is likely to ICE. If so, the assertion shouldn't be enabled in release mode but only if ENABLE_CHECKING is defined. I don't see the logic here. It is certainly not that hard to fix the code to make this assertion trivial, simple loop through all insns in sched_init() will do it. There is a bug and it should get fixed rather than suppressed. Or maybe I misunderstood you? -- Maxim
Re: New assert in haifa-sched.c
Eric Botcazou writes: > > Yes, the assert is really checking exactly that. Several pieces of > > haifa-sched.c assume that the instruction has been recognized during > > scheduler initialization to speed up checking if instruction is normal > > or some kind of use/clobber/asm. > > What happens if an instruction hasn't been recognized upon reaching these > parts of haifa-sched.c? Will it be only mis-scheduled, i.e. will this only > result in inferior, but still correct code? If so, the assertion shouldn't > be enabled in release mode but only if ENABLE_CHECKING is defined. Well, I can't bootstrap as it is so it would be nice if this was fixed regardless of the checking level. (And this is what my patch does in http://gcc.gnu.org/ml/gcc-patches/2008-09/msg00574.html.) Another option would be to completely remove the assert. Adam
Re: New assert in haifa-sched.c
> I don't see the logic here. It is certainly not that hard to fix the > code to make this assertion trivial, simple loop through all insns in > sched_init() will do it. Yes, but apparently nobody is willing to do that at the moment so we'll need to kludge until then and add calls to recog_memoized when we stumble on insns that have not been recognized. But our laziness shouldn't impact the user if we are confident that this cannot result in wrong code. > There is a bug and it should get fixed rather than suppressed. AFAICS nobody has submitted a real fix for this bug. I'm ready to approve Adam's patch but only if the assertion is downgraded to ENABLE_CHECKING. -- Eric Botcazou
Re: Defining a common plugin machinery
Basile STARYNKEVITCH wrote: Hello Diego and all, Diego Novillo wrote: After the FSF gives final approval on the plugin feature, we will need to coordinate towards one common plugin interface for GCC. I don't think we should be adding two different and incompatible plugin harnesses. What exactly did happen on the FSF side after the last GCC summit? I heard nothing more detailed than the Steeering Committee Q&A BOFS and the early draft of some legal licence involving plugins. What happened on the Steering Commitee or legal side since august 2008? Is there any annoucement regarding FSF approving plugins? I am CCing everyone who I know is working on plugin features. Apologies if I missed anyone. I would like to start by understanding what the plugin API should have. What features do we want to support? What should we export? What should be the user interface to incorporate plugin code? At what point in the compilation stage should it kick in? Once we define a common API and then we can take the implementation from the existing branches. Perhaps create a common branch? I would also like to understand what the status and features of the different branches is. The MELT plugin machinery is quite specific in its details, and I don't believe it can be used -in its current form- for other plugins. It really expects the plugin to be a MELT one. From what I remember of the plugin BOFS (but I may be wrong), an easy consensus seems to be that plugins should be loadable thru the command line (probably a -fplugin=foo meaning that some foo.so should be dlopen-ed), that they could take a single string as an argument (using -fplugin-arg=bar) and that plugins add essentially new passes into pass.c - I am happy with such a picture (and could fit MELT easily into it; in its current form MELT is adding a few basilys*passes into passes.c, each having a gate which try using a MELT closure somewhere so is equivalent of testing if the pass is enabled.). Albert Cohen & Grigori Fursin (both in CC) from INRIA have an "Interative Compiler Interface" (a big patch to GCC) which could be helpful to enhance this idea of extending the pass manager. AFAIK, they also have some plugin facility (or maybe I am wrong). Perhaps some ideas or code could be reused for plugins. I have no idea if some general plugin code evolved recently, i.e. if someone is actively working on it. If someone is coding on plugins, please tell the gcc@ list. Hi Guys, We are actively using and maintaining our plugin API at Mozilla. We moderated the plugin discussion at the summit. From our discussion sounds like we need to be able to hook into the middle-end(pass manager) and the frontend. As far as I know nobody has needed anything of the backend yet. We are using the -fplugin syntax that Basile described above to dlopen our plugins. As Basile mentioned, the pass manager API is extremely important. Plugins need to be able to list passes(thus all passes need unique names) and to insert a new pass at arbitrary positions. If I recall correctly, Grigori's work also needs to be able to reorder existing passes. Beyond that, there need to be general callbacks sprinkled throughout the code. At the plugin BoF we decided that something like plugin_callback(EVENT_NAME, void *data) would suit us well. Here EVENT_NAME would be an enum describing the callback. Examples: * PLUGIN_FINISH_STRUCT(data would point at tree_node for the type that gcc just finished building) * PLUGIN_FINISH_GCC(before gcc exits) * PLUGIN_CXX_CP_PRE_GENERICIZE (data is a function body tree_node) for being able to look at fe gimple. * PLUGIN_PLUGIN_SETUP for when it's time to register with the pass manager * other stuff such being notified before particular passes execute(I think Grigori needs this). For my treehydra plugin it is important to land the GTY patch that Diego reviewed. I can start submitting patches for review whenever someone is ready to review them. Our plugin code isn't yet structured exactly as described but I will happily restructure the API to something acceptable and submit stuff for review. See http://developer.mozilla.org/en/Dehydra for more details on Mozilla's plugin work. Cheers, Taras
