A better __builtin_constant_p

2019-06-30 Thread Pip Cet 
I'd like to introduce a new builtin, __builtin_constant_function_p,
with nicer semantics and better performance than __builtin_constant_p.


Background:

I want to define an assert_or_assume() macro that is equivalent to
assume() or assert(), whichever results in better compiled code.

I'd like to use __builtin_constant_p to decide whether
assert_or_assume evaluates this construct:

if (!(CONDITION))
  __builtin_unreachable ();

My current idea is to write:

if (__builtin_constant_p (!(CONDITION) != !(CONDITION)))
  if (!(CONDITION))
__builtin_unreachable ();

This would check that evaluating CONDITION "twice" (in undefined
order) results in the same truth value; if CONDITION includes, for
example, external function calls, we won't be able to prove that, so
the expensive construct would be skipped.  However, for common
conditions such as read-only logical and arithmetic expressions,
!(CONDITION) != !(CONDITION) will be folded to 0, which
__builtin_constant_p can then recognize as a constant.


The Problem:

However, it seems that with trunk GCC, __builtin_constant_p always
returns false for expressions which contain calls to functions not
explicitly marked with attribute((const)), even when those functions
are both inlined and found to allow the const attribute by
-Wsuggest-attr=const.

This is in contrast to macros which work just fine in
__builtin_constant_p's argument.  So, in this case, an inline function
isn't as fast as a macro.

The problem is that __builtin_constant_p must fold its argument very
early, before gimplification, to avoid evaluation of the argument.


My proposed solution:

The idea I'm currently playing around with is to add a new
__builtin_constant_function_p builtin, which folds to 1 as soon if its
argument can be shown, at compile time, to be a pointer to a constant
function.  This can be used in conjunction with inner functions (C
only) to redefine constant_p as:

#define constant_p(EXPR) ({ int inner(void) { return EXPR; };
__builtin_constant_function_p (inner); })

As mentioned, this definition of constant_p would be superior to
__builtin_constant_p while also ensuring the expression itself is
never evaluated.


Patch:

In early test runs, the attached patch appears to work, but I'm
inexperienced when it comes to GCC internals, and I also find that
people often disagree with me for good reasons.


Limitations:

This currently works for C only (using lambdas to expand it to C++
looks like it ought to be possible to me).

Compilation time probably increases significantly, because an inner
function is generated and optimized only to be eventually
discarded. It's also possible this affects code generation in the rest
of the outer function.

This absolutely hasn't been tested enough. I'm attaching my main test
case.

We give up in the fold-builtins pass. It might be a better idea to
wait for the last such pass, if we could.

For very good reasons, this doesn't work as I naively expected it
would for functions marked with attribute((const)), because the
compiler cannot assume that those functions, if called, would return.

Questions:

Am I totally on the wrong track here?

Should we have __builtin_assume?

Should we instead fix our code so forgetting a "const" attribute won't
hurt performance, if it can be inferred?

Should there be a new function attribute to mark functions that may be
assumed by the compiler to return?

Should we fix after-the-fact assume()s to work better?
#include 

extern int nonnegative (int) __attribute__((const));
#ifdef FAST
int nonnegative (int i)
{
  return i >= 0;
}
#endif

int main(int argc, char **argv)
{
  int i = argc;
  int c;
  {
auto int inner(void)
{
  return nonnegative(i) == nonnegative(i);
}

c = __builtin_constant_function_p(inner);
  }

  if (c)
if (!nonnegative(i))
  __builtin_unreachable ();

  printf("%d\n", i & 0x8000);
}

From 5e694e0e35b56caf4469cb516db50608f026c741 Mon Sep 17 00:00:00 2001
From: Pip Cet 
Date: Sun, 30 Jun 2019 12:24:17 +
Subject: [PATCH] Add a __builtin_constant_function_p builtin.

---
 gcc/builtins.c | 78 ++
 gcc/builtins.def   |  1 +
 gcc/expr.c |  4 ++-
 gcc/tree-ssa-ccp.c |  8 +
 4 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 4ecfd49d03c..e1ff327c84b 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -30,6 +30,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree.h"
 #include "memmodel.h"
 #include "gimple.h"
+#include "gimple-iterator.h"
+#include "gimple-walk.h"
 #include "predict.h"
 #include "params.h"
 #include "tm_p.h"
@@ -150,6 +152,7 @@ static tree stabilize_va_list_loc (location_t, tree, int);
 static rtx expand_builtin_expect (tree, rtx)

Re: Clobber REG_CC only for some constraint alternatives?

2020-08-14 Thread Pip Cet via Gcc 
On Fri, Aug 14, 2020 at 3:33 PM Matt Wette via Gcc  wrote:
> Happy to see someone working this.   Are you starting with one CC mode?

I'm also working on this (mostly at bug#92792), and so far am using
two modes: the general reg:CC mode for proper comparison insns, and
CCNZ for optimization in the CC-setting variants produced by the
cmpelim pass.

> I noticed that the current CC0 implementation seems to effectively use
> several modes.  For example, one for use of the t flag.

As far as I can tell, the current code emits T setters and getters
together as one insn, even going to the trouble of setting T
redundantly when it already has been set. Am I missing something?

Re: Clobber REG_CC only for some constraint alternatives?

2020-08-14 Thread Pip Cet via Gcc 
On Fri, Aug 14, 2020 at 4:24 PM Segher Boessenkool
 wrote:
> On Fri, Aug 14, 2020 at 04:46:59PM +0530, Senthil Kumar Selvaraj via Gcc 
> wrote:
> > (define_insn "*mov_insn_noclobber_flags"
> >   [(set (match_operand:ALL1 0 "nonimmediate_operand" "=r,d   ,q,r")
> > (match_operand:ALL1 1 "nox_general_operand"   "r,n Ynn,r,q"))
> >(clobber (const_int 0))]
>
> This is not correct, clobbers like that are not defined RTL, and are
> actually used internally (by combine at least), so this will confuse
> that.
>
> If you want to say some alternative does not clobber anything, just use
> the constraint "X" for that alternative.

Just to clarify, such clobbers would still show up in RTL, right?
Because some passes, such as cmpelim, do not currently appear to deal
very well with extra clobbers, so that might be a problem.

What I'm currently doing is this:

(define_split
  [(set (match_operand 0 "nonimmediate_operand")
(match_operand 1 "nox_general_operand"))]
  "reload_completed && mov_clobbers_cc (insn, operands)"
  [(parallel [(set (match_dup 0) (match_dup 1))
  (clobber (reg:CC REG_CC))])])

which, if I understand correctly, introduces the parallel-clobber
expression only when necessary, at the same time as post-reload
splitters introduce REG_CC references. Is that correct?

Thanks
Pip

Re: Clobber REG_CC only for some constraint alternatives?

2020-08-15 Thread Pip Cet via Gcc 
On Sat, Aug 15, 2020 at 12:30 AM Segher Boessenkool
 wrote:
> On Fri, Aug 14, 2020 at 05:47:02PM +0000, Pip Cet wrote:
> > On Fri, Aug 14, 2020 at 4:24 PM Segher Boessenkool
> >  wrote:
> > > If you want to say some alternative does not clobber anything, just use
> > > the constraint "X" for that alternative.
> >
> > Just to clarify, such clobbers would still show up in RTL, right?
>
> Yes, as
>
>   (clobber (scratch:CC))
>
> (or whatever the mode is).  No register will be allocated to it.  You
> can do a define_split splitting it into the form without clobber, if
> you want?  (You can "split" to just one insn fine.)  It's neatest when
> written as a define_insn_and_split.

That does sound like a neat solution for leaving the current patterns
largely intact, thanks! I'll try both variants, both and without the
define_split.

> > What I'm currently doing is this:
> >
> > (define_split
> >   [(set (match_operand 0 "nonimmediate_operand")
> > (match_operand 1 "nox_general_operand"))]
> >   "reload_completed && mov_clobbers_cc (insn, operands)"
> >   [(parallel [(set (match_dup 0) (match_dup 1))
> >   (clobber (reg:CC REG_CC))])])
> >
> > which, if I understand correctly, introduces the parallel-clobber
> > expression only when necessary, at the same time as post-reload
> > splitters introduce REG_CC references. Is that correct?
>
> Yes.  And this will work correctly if somehow you ensure that REG_CC
> isn't live anywhere you introduce such a clobber.

IIUC, the idea is that references to REG_CC, except for clobbers, are
only introduced in the post-reload split pass, so it cannot be live
before our define_split runs. Does that make sense?

Re: Clobber REG_CC only for some constraint alternatives?

2020-08-16 Thread Pip Cet via Gcc 
On Sun, Aug 16, 2020 at 12:50 AM Segher Boessenkool
 wrote:
> On Sat, Aug 15, 2020 at 10:18:27AM +0000, Pip Cet wrote:
> > > > What I'm currently doing is this:
> > > >
> > > > (define_split
> > > >   [(set (match_operand 0 "nonimmediate_operand")
> > > > (match_operand 1 "nox_general_operand"))]
> > > >   "reload_completed && mov_clobbers_cc (insn, operands)"
> > > >   [(parallel [(set (match_dup 0) (match_dup 1))
> > > >   (clobber (reg:CC REG_CC))])])
> > > >
> > > > which, if I understand correctly, introduces the parallel-clobber
> > > > expression only when necessary, at the same time as post-reload
> > > > splitters introduce REG_CC references. Is that correct?
> > >
> > > Yes.  And this will work correctly if somehow you ensure that REG_CC
> > > isn't live anywhere you introduce such a clobber.
> >
> > IIUC, the idea is that references to REG_CC, except for clobbers, are
> > only introduced in the post-reload split pass, so it cannot be live
> > before our define_split runs. Does that make sense?
>
> Yes, it does.  It has some huge restrictions (using the reg in inline
> assembler can never work reliably, for example, so you'll have to
> disallow that).

Is there any approach that doesn't suffer from that problem? My
understanding was that we need to allow reload to insert CC-clobbering
insns on this (and many other) architectures, and that there are so
many places where reload might choose to do so (including before and
after inline asm) that using the register prior to reload just isn't
possible. I'd be glad to be wrong, though :-)

Is it true that reload chooses which constraint alternative is used
for each insn? Is that information available somehow to post-reload
splits? The problem is that the "X" constraint matches whatever's
there already, and doesn't replace it with the (scratch:CC) expression
that would work, so I can't rewrite those insns not to clobber the CC
reg.

For example, here's what I currently have:

(define_expand "mov"
  [(parallel [(set (match_operand:MOVMODE 0 "nonimmediate_operand" "")
   (match_operand:MOVMODE 1 "general_operand" ""))
  (clobber (reg:CC REG_CC))])]
  ...)

(define_insn "mov_insn"
   [(set (match_operand:ALL1 0 "nonimmediate_operand" "=r,r  ,d,Qm
  ,r ,q,r,*r")
(match_operand:ALL1 1 "nox_general_operand"   "r,Y00,n Ynn,r
Y00,Qm,r,q,i"))
   (clobber (match_scratch:CC 2 "=X,c,X,c,c,X,X,c"))]
  ...)

That works, but it results in an incorrect CC clobber for, say,
register-to-register movqi. For that, I'd need something like

(define_split
  [(parallel [(set (match_operand:ALL1 0 "nonimmediate_operand")
 (match_operand:ALL1 1 "nox_general_operand"))
  (clobber (reg:CC REG_CC))])]
  "reload_completed && REG_P (operands[0]) && REG_P (operands[1])"
  [(parallel [(set (match_dup 0)
   (match_dup 1))
  (clobber (scratch:CC))])])

and so on, for all four constraint alternatives that don't actually
clobber REG_CC (and also for a fifth which only rarely clobbers
REG_CC). That's duplicated code, of course.

All this is at least somewhat academic: the code produced isn't
drastically better after my cc conversion, but it's not usually any
worse, and I'm still looking at assembler examples that are pessimized
a little to find out how to fix them...

> And earlier passes (like combine) cannot optimise this,

I'm not sure what "this" refers to: my understanding is that the idea
is to let combine see CC-free code, or code with CC clobbers only,
which it can then optimize, and only add "real" CC references after
reload, so many optimizations should just work.

> But it is a pretty straightforward way to move from CC0 to the
> modern world!

With the limitations of the reload pass being as they are, I don't
really see a dramatically better alternative. I suppose we could
explicitly save and restore CC flags around insns emitted when
reload_in_progress is true, to simulate non-CC-clobbering add/mov insn
patterns? That sounds like it would reduce code quality a lot, unless
great care were taken to make sure all of the save/restore CC flags
insns were optimized away in later passes.

In any case, thanks for the answers so far!

Pip

Re: Clobber REG_CC only for some constraint alternatives?

2020-08-17 Thread Pip Cet via Gcc 
On Mon, Aug 17, 2020 at 7:31 AM Senthil Kumar Selvaraj
 wrote:
> > (define_split
> >   [(parallel [(set (match_operand:ALL1 0 "nonimmediate_operand")
> >  (match_operand:ALL1 1 "nox_general_operand"))
> >   (clobber (reg:CC REG_CC))])]
> >   "reload_completed && REG_P (operands[0]) && REG_P (operands[1])"
> >   [(parallel [(set (match_dup 0)
> >(match_dup 1))
> >   (clobber (scratch:CC))])])
> >
> > and so on, for all four constraint alternatives that don't actually
> > clobber REG_CC (and also for a fifth which only rarely clobbers
> > REG_CC). That's duplicated code, of course.
>
> The (setattr "cc" ...) that is currently present for all
> patterns accounts for the constraint alternatives,so  using
> get_attr_cc to split to a (clobber) of either cc_reg_rtx or a
> gen_rtx_SCRATCH (CCmode) appears to work.

Thanks! Using an insn attribute is actually what I ended up doing
(https://github.com/pipcet/gcc/commit/d4509afae9238e0ade4d3e1e97dd8577dae96115)
:-)

It's still confusing, IMHO, that insn attributes (but not the
get_attr_alternative attribute which is mentioned in the
documentation) are available when which_alternative is not.

> (define_insn "*mov_insn"
>   [(set (match_operand:ALL1 0 "nonimmediate_operand" "=r,d,Qm   ,r 
> ,q,r,*r")
> (match_operand:ALL1 1 "nox_general_operand"   "r Y00,n Ynn,r 
> Y00,Qm,r,q,i"))
>(clobber (match_scratch:CC 2  "=X,X ,c   ,c 
> ,X,X,c"))]

Hmm. Technically, of course, clearing register 0 (a special case of
the first alternative) would clobber the flags, but as it happens, the
rewrite above produces the right clobber expression which is simply
accepted by the "X"... I'm not sure whether anything else is trying to
recognize such insns, but as it stands that define_insn would
recognize the incorrect insn:

[(set (reg:QI 0) (const_int 0))
 (clobber (scratch:CC))]

Re: Clobber REG_CC only for some constraint alternatives?

2020-08-20 Thread Pip Cet via Gcc 
On Tue, Aug 18, 2020 at 6:52 AM Senthil Kumar Selvaraj
 wrote:
> > recognize such insns, but as it stands that define_insn would
> > recognize the incorrect insn:
> >
> > [(set (reg:QI 0) (const_int 0))
> >  (clobber (scratch:CC))]
>
> get_cc_reg_clobber_rtx also looks at the insn itself (i.e. whatever
> the erstwhile NOTICE_UPDATE_CC hook did), so if the cc attribute is LDI,
> and operands[1] is const0_rtx, it would return cc_reg_rtx as the clobber reg.
>
> AFAICT, some passes after reload verify if operands match constraints,
> and that would work in this case - I'm not sure if the pattern you
> mentioned could show up, outside of wrong splitters/peepholes in the md file.

I don't think they can, but it's still another case of lying to GCC.

At the very least, output_movqi should assert that it isn't asked to
produce code for this situation.

> Another approach would be to conservatively use a 'c' constraint and
> clobber REG_CC for all reg-reg moves.

I'd prefer splitting the constraint alternatives to have one clear-r0
alternative, an ldi alternative, and a clear -r1_31 alternative.

As for define_subst, is it really worth it? If I'm reading the
documentation correctly, it's not powerful enough to deal with scratch
operands on its own, so we'd probably need three or four variants of
define_subst just to handle those cases.

I'm probably missing something obvious, but what's the reason for
keeping the CC-clobbering post-reload splitter when we already have a
CC-setting one? Are there significant post-reload optimizations that
can deal with a clobber but not an arbitrary assignment to CC? If so,
wouldn't it be better to fix those optimizations?

(There are at least some pre-reload optimizations that don't work with
the CC-clobbering insn patterns. lower_subreg.c's simple_move, for
example, recognizes only two-operand sets. This resulted in pessimized
code, but is easy to fix.)

Re: Clobber REG_CC only for some constraint alternatives?

2020-08-26 Thread Pip Cet via Gcc 
On Mon, Aug 24, 2020 at 6:18 PM Jeff Law  wrote:
> > The post-reload splitter introduces the clobber. The wiki
> > suggests that approach if most insns clobber REG_CC, perhaps because of
> > the missed optimizations you describe below?
> If most patterns set/clobber the flags, then yes, it's slightly better to only
> expose them after reload.   Various passes that directly grub through RTL 
> rather
> than using helpers like single_set will optimize things better.

I think I made it to the next pitfall :-)

The cmpelim pass tries to recognize cc-setting variants of insns.
Whether or not there is one (i.e. whether or not the insn should be
recognized) depends on the "cc" attribute, which depends on which
alternative is used. So I did the obvious thing, and put a condition
in the define_insn which depends on get_cc_attr (insn). But
get_cc_attr() tries to recognize the insn, so we recurse indefinitely
and die with a segfault.

Things appear to work with a somewhat subtle hack: we recognize that a
false positive from the inner recognition isn't harmful, because the
outer condition will still catch invalid cases.

  static int recurse = 0;
  if (recurse)
return gen_rtx_REG (CCmode, REG_CC); // causes the insn to be recognized
  recurse++;
  int old_insn_code = INSN_CODE (insn);
  enum attr_cc cc = get_attr_cc (insn);
  INSN_CODE (insn) = old_insn_code;
  recurse--;

But surely that's not the right way?

Note that whether there is a CC-setting variant depends not just on
the "cc" attr, but also on the precise operands for some values of the
"cc" attr, which requires hairy C code to figure out.

Is it possible to avoid this situation by avoiding constraint
alternatives, and defining insns separately for each of them?

#line directives in generated C files

2020-08-27 Thread Pip Cet via Gcc 
I may be missing an obvious workaround, but it seems we currently emit
a #line directive when including lines from machine description files
in C files, but never emit a second directive when switching back to
the generated C file. This makes stepping through the backend in gdb
somewhat painful, because gdb thinks it should display lines from the
md file even after leaving the included fragment.

The attached patch is a proof of concept which unconditionally emits a
line containing "/* #unline */" after such fragments, and runs all
generated C files through a trivial filter which replaces those lines
by #line directives pointing back to the original file. It appears to
work.
From c1c85da2b5d029b730f98906abda6c73bb7352d3 Mon Sep 17 00:00:00 2001
From: Pip Cet 
Date: Thu, 27 Aug 2020 22:02:40 +
Subject: [PATCH] Add paired #line directives pointing back to the generated C
 file.

---
 gcc/Makefile.in | 15 ---
 gcc/genemit.c   |  1 +
 gcc/genline.c   | 36 
 gcc/read-md.c   | 19 +++
 gcc/read-md.h   |  2 ++
 5 files changed, 66 insertions(+), 7 deletions(-)
 create mode 100644 gcc/genline.c

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 79e854aa938..fb03031d038 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -2354,16 +2354,16 @@ $(simple_rtl_generated_c:insn-%.c=s-%): s-%: insn-conditions.md
 
 $(simple_generated_h): insn-%.h: s-%; @true
 
-$(simple_generated_h:insn-%.h=s-%): s-%: build/gen%$(build_exeext)
+$(simple_generated_h:insn-%.h=s-%): s-%: build/gen%$(build_exeext) build/genline$(build_exeext)
 	$(RUN_GEN) build/gen$*$(build_exeext) $(md_file) \
-	  $(filter insn-conditions.md,$^) > tmp-$*.h
+	  $(filter insn-conditions.md,$^) | $(RUN_GEN) build/genline insn-$*.h > tmp-$*.h
 	$(SHELL) $(srcdir)/../move-if-change tmp-$*.h insn-$*.h
 	$(STAMP) s-$*
 
 $(simple_generated_c): insn-%.c: s-%; @true
-$(simple_generated_c:insn-%.c=s-%): s-%: build/gen%$(build_exeext)
+$(simple_generated_c:insn-%.c=s-%): s-%: build/gen%$(build_exeext) build/genline
 	$(RUN_GEN) build/gen$*$(build_exeext) $(md_file) \
-	  $(filter insn-conditions.md,$^) > tmp-$*.c
+	  $(filter insn-conditions.md,$^) | $(RUN_GEN) build/genline insn-$*.c > tmp-$*.c
 	$(SHELL) $(srcdir)/../move-if-change tmp-$*.c insn-$*.c
 	$(STAMP) s-$*
 
@@ -2397,8 +2397,8 @@ s-opinit: $(MD_DEPS) build/genopinit$(build_exeext) insn-conditions.md
 
 # gencondmd doesn't use the standard naming convention.
 build/gencondmd.c: s-conditions; @true
-s-conditions: $(MD_DEPS) build/genconditions$(build_exeext)
-	$(RUN_GEN) build/genconditions$(build_exeext) $(md_file) > tmp-condmd.c
+s-conditions: $(MD_DEPS) build/genconditions$(build_exeext) build/genline$(build_exeext)
+	$(RUN_GEN) build/genconditions$(build_exeext) $(md_file) | $(RUN_GEN) build/genline$(build_exeext) gencondmd.c > tmp-condmd.c
 	$(SHELL) $(srcdir)/../move-if-change tmp-condmd.c build/gencondmd.c
 	$(STAMP) s-conditions
 
@@ -2783,6 +2783,7 @@ build/genextract.o : genextract.c $(RTL_BASE_H) $(BCONFIG_H)		\
   $(SYSTEM_H) $(CORETYPES_H) $(GTM_H) errors.h $(READ_MD_H) $(GENSUPPORT_H)
 build/genflags.o : genflags.c $(RTL_BASE_H) $(OBSTACK_H) $(BCONFIG_H)	\
   $(SYSTEM_H) $(CORETYPES_H) $(GTM_H) errors.h $(READ_MD_H) $(GENSUPPORT_H)
+build/genline.o : genline.c
 build/gentarget-def.o : gentarget-def.c $(BCONFIG_H) $(SYSTEM_H)	\
   $(CORETYPES_H) $(GTM_H) $(RTL_BASE_H) errors.h $(READ_MD_H)		\
   $(GENSUPPORT_H) $(HASH_TABLE_H) target-insns.def
@@ -2867,7 +2868,7 @@ genprogerr = $(genprogmd) genrtl modes gtype hooks cfn-macros condmd
 $(genprogerr:%=build/gen%$(build_exeext)): $(BUILD_ERRORS)
 
 # Remaining build programs.
-genprog = $(genprogerr) check checksum match
+genprog = $(genprogerr) check checksum line match
 
 # These programs need libs over and above what they get from the above list.
 build/genautomata$(build_exeext) : BUILD_LIBS += -lm
diff --git a/gcc/genemit.c b/gcc/genemit.c
index 84d07d388ee..5e4b82fe3de 100644
--- a/gcc/genemit.c
+++ b/gcc/genemit.c
@@ -310,6 +310,7 @@ emit_c_code (const char *code, bool can_fail_p, const char *name)
 
   rtx_reader_ptr->print_md_ptr_loc (code);
   printf ("%s\n", code);
+  rtx_reader_ptr->print_md_ptr_unloc (code);
 
   printf ("#undef DONE\n");
   printf ("#undef FAIL\n");
diff --git a/gcc/genline.c b/gcc/genline.c
new file mode 100644
index 000..49ecf27a76c
--- /dev/null
+++ b/gcc/genline.c
@@ -0,0 +1,36 @@
+#include 
+
+int main(int argc, char **argv)
+{
+  if (argc < 2)
+return -1;
+
+  const char *state = "/* #unline */\n";
+  const char *p = state;
+  int line = 1;
+
+  int c;
+  do
+{
+  c = getchar ();
+  if (c == '\n')
+	line++;
+  if (!*p)
+	{
+	  printf ("#line %d \"%s\"\n", line, argv[1]);
+	  p = state;
+	}
+  if (*p == c)
+	p++;
+  else
+	{
+	  printf ("%.*s", (int)(p - state), state);
+	  p = state;
+

Re: #line directives in generated C files

2020-09-04 Thread Pip Cet via Gcc 
On Thu, Sep 3, 2020 at 8:19 PM Hans-Peter Nilsson  wrote:
> On Thu, 27 Aug 2020, Pip Cet via Gcc wrote:
> > I may be missing an obvious workaround, but it seems we currently emit
> > a #line directive when including lines from machine description files
> > in C files, but never emit a second directive when switching back to
> > the generated C file. This makes stepping through the backend in gdb
>
> Thanks for taking on this!

Thanks for the encouragement!

> IMHO stepping into the .md really isn't helpful.  Even a pattern
> name in a comment in the generated code would be better.

I think it is helpful, FWIW, to be able to set a breakpoint on an md
condition or in the preparation code (in conjunction with setting
CXXFLAGS to "-O0 -g3" or similar), but since that's not a "normal"
compilation it'd be acceptable to specify an extra switch for this
feature. That would mean my genline.c program wouldn't have to run
except in those constellations...

Re: Deprecating complex integer types in GCC

2024-08-19 Thread Pip Cet via Gcc 
"Andrew Pinski \(QUIC\) via Gcc"  writes:

> Deprecating complex integer types in GCC seems like a good idea. There
> has been issues with division with them before and it was raised back
> them maybe we should deprecate their support.
> The previous discussion about deprecating them can be found
> https://gcc.gnu.org/legacy-ml/gcc/2001-11/msg00790.html and
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=2995 .
> They were never standardized nor I doubt many folks use them.
> This also came up since clang/LLVM has similar issues to GCC previously had 
> too: https://github.com/llvm/llvm-project/issues/104738 .
> I doubt many folks use them or even know it is an supported GNU extension 
> either so deprecating them for GCC 15 seems like a decent idea.
>
> Any thoughts on this? And possibly removing support in GCC 16?

Just as an anecdote, they caused trouble for Emacs developers not so
long ago:

https://lists.gnu.org/archive/html/emacs-devel/2024-07/msg4.html

As changing 0 (or 1) to "i" is such a common C idiom, that typo is not
going to be rare, so adding, at least, a warning option analogous to
clang's (undocumented?) -Wgnu-imaginary-constant
(https://lists.gnu.org/archive/html/emacs-devel/2024-07/msg00030.html)
would be a good idea.

Gaussian integers are useful sometimes, but I'm not so sure about
Gaussian ints, truncated to 32 or 64 bits.  I suspect most users would
be much happier with unlimited-precision libraries that avoid the
truncation.

Pip Cet