On Wed, 30 Oct 2013, Zhenqiang Chen wrote: > > -----Original Message----- > > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > > ow...@gcc.gnu.org] On Behalf Of Hans-Peter Nilsson > > One thing I don't see other people mentioning, is that this patch has just > too > > much code inside #ifdef HAVE_ccmp ... #endif. > > > > I couldn't actually find the part that *requires* that, i.e. > > code that does something like gen_ccmp (...) but maybe it's there. > > In the arm.md, I define "(define_expand "ccmp" ...". It will be transferred > to function gen_ccmp (...). And it will define HAVE_ccmp.
I know. I have not misunderstood that machinery. The point again, is that inside the "#ifdef HAVE_ccmp" there should contain as few lines as possible. > In the updated patch, I add two backends hooks to generate conditional > compare instruction. And that is good. Where you use hooks, we don't need the #ifdef HAVE_ccmp. That is one of the major points with hooks. > > Where needed and where the conditioned code is more than a few lines, > > such code is preferably transformed into "if (0)":d code using constructs > like > > that at builtins.c:5354: > > > > #ifndef HAVE_atomic_clear > > # define HAVE_atomic_clear 0 > > # define gen_atomic_clear(x,y) (gcc_unreachable (), NULL_RTX) #endif > > > > Right, this causes dead code, but for maintenance it's *much* better than > > when only a fraction of the code being compiled for other targets. (Also, > the > > HAVE_ccmp is automatically generated when the target has ccmp instruction. Exactly like with my example; "atomic_clear" is generated from an insn pattern. ...or actually, it appears it's *supposed* to be, but I don't see any target defining it and it's not special in genconfig.c and not even documented in md.texi so that might be code dead everywhere! We have atomic_store but no target defines atomic_clear; certainly not without a mode. My bad for using a bad example. For a better, "live" example, see function.c:4665 same construct as above but with HAVE_stack_protect_set. > The code segments in #ifdef HAVE_ccmp ... #endif will *not* be compiled for > other targets, which do not define ccmp instruction. I think you misunderstood: *that is the problem*. We want as little as possible code to be conditional and inside #if... preprocessor macros; we want it to use hooks and "if (0)" code. brgds, H-P