---
v2 -> v3:
	- got rid of column numbers and use the arguments of loc instructions
	  for file number and line number
	- got rid of -g flag
	- file directives don't modify global state anymore, they're just
	  passed on to assembly

 all.h        |  2 +-
 amd64/emit.c |  4 ++++
 amd64/isel.c |  1 +
 arm64/emit.c |  4 ++++
 arm64/isel.c |  8 +++++---
 main.c       |  2 +-
 ops.h        |  3 +++
 parse.c      | 15 +++++++++++++--
 rv64/emit.c  |  4 ++++
 rv64/isel.c  |  8 +++++---
 tools/lexh.c |  4 ++--
 11 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/all.h b/all.h
index c8ec93c..8b63075 100644
--- a/all.h
+++ b/all.h
@@ -469,7 +469,7 @@ bshas(BSet *bs, uint elt)
 
 /* parse.c */
 extern Op optab[NOp];
-void parse(FILE *, char *, void (Dat *), void (Fn *));
+void parse(FILE *, char *, FILE *, void (Dat *), void (Fn *));
 void printfn(Fn *, FILE *);
 void printref(Ref, Fn *, FILE *);
 void err(char *, ...) __attribute__((noreturn));
diff --git a/amd64/emit.c b/amd64/emit.c
index b8e9e8e..7fb785e 100644
--- a/amd64/emit.c
+++ b/amd64/emit.c
@@ -519,6 +519,10 @@ emitins(Ins i, Fn *fn, FILE *f)
 		emitcopy(i.arg[0], i.arg[1], i.cls, fn, f);
 		emitcopy(i.arg[1], TMP(XMM0+15), i.cls, fn, f);
 		break;
+	case Oloc:
+		fprintf(f, "\t.loc %ld %ld\n", fn->con[i.arg[0].val].bits.i,
+			fn->con[i.arg[1].val].bits.i);
+		break;
 	}
 }
 
diff --git a/amd64/isel.c b/amd64/isel.c
index 5a64429..54aba26 100644
--- a/amd64/isel.c
+++ b/amd64/isel.c
@@ -367,6 +367,7 @@ sel(Ins i, ANum *an, Fn *fn)
 	case Oexts:
 	case Otruncd:
 	case Ocast:
+	case Oloc:
 	case_OExt:
 Emit:
 		emiti(i);
diff --git a/arm64/emit.c b/arm64/emit.c
index 2506eea..4c9cb98 100644
--- a/arm64/emit.c
+++ b/arm64/emit.c
@@ -390,6 +390,10 @@ emitins(Ins *i, E *e)
 		if (!req(i->to, R))
 			emitf("mov %=, sp", i, e);
 		break;
+	case Oloc:
+		fprintf(e->f, "\t.loc %ld %ld\n", e->fn->con[i->arg[0].val].bits.i,
+			e->fn->con[i->arg[1].val].bits.i);
+		break;
 	}
 }
 
diff --git a/arm64/isel.c b/arm64/isel.c
index bb5c12b..e30be96 100644
--- a/arm64/isel.c
+++ b/arm64/isel.c
@@ -180,9 +180,11 @@ sel(Ins i, Fn *fn)
 	}
 	if (i.op != Onop) {
 		emiti(i);
-		iarg = curi->arg; /* fixarg() can change curi */
-		fixarg(&iarg[0], argcls(&i, 0), 0, fn);
-		fixarg(&iarg[1], argcls(&i, 1), 0, fn);
+		if (i.op != Oloc) {
+			iarg = curi->arg; /* fixarg() can change curi */
+			fixarg(&iarg[0], argcls(&i, 0), 0, fn);
+			fixarg(&iarg[1], argcls(&i, 1), 0, fn);
+		}
 	}
 }
 
diff --git a/main.c b/main.c
index 2ff9b9c..387e808 100644
--- a/main.c
+++ b/main.c
@@ -184,7 +184,7 @@ main(int ac, char *av[])
 				exit(1);
 			}
 		}
-		parse(inf, f, data, func);
+		parse(inf, f, outf, data, func);
 		fclose(inf);
 	} while (++optind < ac);
 
diff --git a/ops.h b/ops.h
index 285bc5c..c5eb21e 100644
--- a/ops.h
+++ b/ops.h
@@ -121,6 +121,9 @@ O(vastart, T(m,e,e,e, x,e,e,e), 0) X(0, 0, 0) V(0)
 
 O(copy,    T(w,l,s,d, x,x,x,x), 0) X(0, 0, 1) V(0)
 
+/* Debug info */
+O(loc,     T(w,l,s,d, w,l,s,d), 0) X(0, 0, 1) V(0)
+
 
 /****************************************/
 /* INTERNAL OPERATIONS (keep nop first) */
diff --git a/parse.c b/parse.c
index 1912c8b..bbeff0b 100644
--- a/parse.c
+++ b/parse.c
@@ -43,6 +43,7 @@ enum {
 	Tdata,
 	Tsection,
 	Talign,
+	Tfile,
 	Tl,
 	Tw,
 	Th,
@@ -93,6 +94,7 @@ static char *kwmap[Ntok] = {
 	[Tdata] = "data",
 	[Tsection] = "section",
 	[Talign] = "align",
+	[Tfile] = "file",
 	[Tl] = "l",
 	[Tw] = "w",
 	[Th] = "h",
@@ -109,7 +111,7 @@ enum {
 	TMask = 16383, /* for temps hash */
 	BMask = 8191, /* for blocks hash */
 
-	K = 5041217, /* found using tools/lexh.c */
+	K = 6326733, /* found using tools/lexh.c */
 	M = 23,
 };
 
@@ -552,6 +554,7 @@ parseline(PState ps)
 		if (isstore(t)) {
 		case Tcall:
 		case Ovastart:
+		case Oloc:
 			/* operations without result */
 			r = R;
 			k = Kw;
@@ -1091,7 +1094,7 @@ parselnk(Lnk *lnk)
 }
 
 void
-parse(FILE *f, char *path, void data(Dat *), void func(Fn *))
+parse(FILE *f, char *path, FILE *outf, void data(Dat *), void func(Fn *))
 {
 	Lnk lnk;
 	uint n;
@@ -1108,6 +1111,14 @@ parse(FILE *f, char *path, void data(Dat *), void func(Fn *))
 		switch (parselnk(&lnk)) {
 		default:
 			err("top-level definition expected");
+		case Tfile:
+			expect(Tint);
+			if (tokval.num <= 0)
+				err("expected positive integer");
+			n = tokval.num;
+			expect(Tstr);
+			fprintf(outf, ".file %u %s\n", n, tokval.str);
+			break;
 		case Tfunc:
 			func(parsefn(&lnk));
 			break;
diff --git a/rv64/emit.c b/rv64/emit.c
index 2656c60..27d4e3b 100644
--- a/rv64/emit.c
+++ b/rv64/emit.c
@@ -377,6 +377,10 @@ emitins(Ins *i, Fn *fn, FILE *f)
 		if (!req(i->to, R))
 			emitf("mv %=, sp", i, fn, f);
 		break;
+	case Oloc:
+		fprintf(f, "\t.loc %ld %ld\n", fn->con[i->arg[0].val].bits.i,
+			fn->con[i->arg[1].val].bits.i);
+		break;
 	}
 }
 
diff --git a/rv64/isel.c b/rv64/isel.c
index e41578b..d877671 100644
--- a/rv64/isel.c
+++ b/rv64/isel.c
@@ -189,9 +189,11 @@ sel(Ins i, Fn *fn)
 	}
 	if (i.op != Onop) {
 		emiti(i);
-		i0 = curi; /* fixarg() can change curi */
-		fixarg(&i0->arg[0], argcls(&i, 0), i0, fn);
-		fixarg(&i0->arg[1], argcls(&i, 1), i0, fn);
+		if (i.op != Oloc) {
+			i0 = curi; /* fixarg() can change curi */
+			fixarg(&i0->arg[0], argcls(&i, 0), i0, fn);
+			fixarg(&i0->arg[1], argcls(&i, 1), i0, fn);
+		}
 	}
 }
 
diff --git a/tools/lexh.c b/tools/lexh.c
index 8d0af21..7024197 100644
--- a/tools/lexh.c
+++ b/tools/lexh.c
@@ -23,10 +23,10 @@ char *tok[] = {
 	"ceql", "cnel", "cles", "clts", "cgts", "cges",
 	"cnes", "ceqs", "cos", "cuos", "cled", "cltd",
 	"cgtd", "cged", "cned", "ceqd", "cod", "cuod",
-	"vaarg", "vastart", "...", "env",
+	"vaarg", "vastart", "...", "env", "loc",
 
 	"call", "phi", "jmp", "jnz", "ret", "export",
-	"function", "type", "data", "section", "align",
+	"function", "type", "data", "section", "align", "file",
 	"l", "w", "h", "b", "d", "s", "z", "loadw", "loadl",
 	"loads", "loadd", "alloc1", "alloc2",
 
-- 
2.36.1

Hi,

Thanks for your work. I think your patch is getting close to
being merge-able in master so that folks can try it out and
provide feedback. I will work on that after the thread-local
storage patches from Drew.

I'm not too keen on the syntax 'file' at toplevel does not
really look like debug. Same applies to the 'loc' operation.

How is 'file' supposed to be used? Would it make sense to
include it in functions' linkage?

I provided some cosmetic feedback on your patch below.

On Sat, Aug 27, 2022, at 17:44, Bor Grošelj Simić wrote:
> ---
> v2 -> v3:
> 	- got rid of column numbers and use the arguments of loc instructions
> 	  for file number and line number
> 	- got rid of -g flag
> 	- file directives don't modify global state anymore, they're just
> 	  passed on to assembly
>
>  all.h        |  2 +-
>  amd64/emit.c |  4 ++++
>  amd64/isel.c |  1 +
>  arm64/emit.c |  4 ++++
>  arm64/isel.c |  8 +++++---
>  main.c       |  2 +-
>  ops.h        |  3 +++
>  parse.c      | 15 +++++++++++++--
>  rv64/emit.c  |  4 ++++
>  rv64/isel.c  |  8 +++++---
>  tools/lexh.c |  4 ++--
>  11 files changed, 43 insertions(+), 12 deletions(-)
>
> diff --git a/all.h b/all.h
> index c8ec93c..8b63075 100644
> --- a/all.h
> +++ b/all.h
> @@ -469,7 +469,7 @@ bshas(BSet *bs, uint elt)
> 
>  /* parse.c */
>  extern Op optab[NOp];
> -void parse(FILE *, char *, void (Dat *), void (Fn *));
> +void parse(FILE *, char *, FILE *, void (Dat *), void (Fn *));
>  void printfn(Fn *, FILE *);
>  void printref(Ref, Fn *, FILE *);
>  void err(char *, ...) __attribute__((noreturn));
> diff --git a/amd64/emit.c b/amd64/emit.c
> index b8e9e8e..7fb785e 100644
> --- a/amd64/emit.c
> +++ b/amd64/emit.c
> @@ -519,6 +519,10 @@ emitins(Ins i, Fn *fn, FILE *f)
>  		emitcopy(i.arg[0], i.arg[1], i.cls, fn, f);
>  		emitcopy(i.arg[1], TMP(XMM0+15), i.cls, fn, f);
>  		break;
> +	case Oloc:
> +		fprintf(f, "\t.loc %ld %ld\n", fn->con[i.arg[0].val].bits.i,
> +			fn->con[i.arg[1].val].bits.i);
> +		break;

This should be in 'gas.c', shared between backends rather
than repeated.

>  	if (i.op != Onop) {
>  		emiti(i);

You are now in the default case, that's not a good
place to deal with Oloc; you should deal with it
earlier.

> +		case Tfile:
> +			expect(Tint);
> +			if (tokval.num <= 0)
> +				err("expected positive integer");
> +			n = tokval.num;
> +			expect(Tstr);
> +			fprintf(outf, ".file %u %s\n", n, tokval.str);
> +			break;

You should call into 'gas.c' here as well.

>  	if (i.op != Onop) {
>  		emiti(i);
> -		i0 = curi; /* fixarg() can change curi */
> -		fixarg(&i0->arg[0], argcls(&i, 0), i0, fn);
> -		fixarg(&i0->arg[1], argcls(&i, 1), i0, fn);
> +		if (i.op != Oloc) {
> +			i0 = curi; /* fixarg() can change curi */
> +			fixarg(&i0->arg[0], argcls(&i, 0), i0, fn);
> +			fixarg(&i0->arg[1], argcls(&i, 1), i0, fn);
> +		}

Same thing here.

> I'm not too keen on the syntax 'file' at toplevel does not
> really look like debug. Same applies to the 'loc' operation.

The names are copied from gas commands they are mimicking.
I'm happy to change them to something you find more suitable.

Those two are not the only additions to QBE syntax that will be needed
for conveying debug information to gas, so in the long term it may make
sense to introduce some special designation for debug info related
commands. (like LLVM has metadata-related command things prefixed with
!)

> How is 'file' supposed to be used? Would it make sense to
> include it in functions' linkage?

`file 1 "main.c"` just means that instructions annotated with `loc 1, x`
originate from that file. It imitates the gas .file directive and isn't
tied to any particular function. The proposed harec patch that makes use
of this functionality just puts all file commands at the beginning of
generated IR for example.

This was written 2 months ago but was never sent for some reason:

> Hi,
>
> Thanks for your work. I think your patch is getting close to
> being merge-able in master so that folks can try it out and
> provide feedback. I will work on that after the thread-local
> storage patches from Drew.
>
> I'm not too keen on the syntax 'file' at toplevel does not
> really look like debug. Same applies to the 'loc' operation.

The names are taken directly from gas, because the functionality is taken
directly from gas too - .file maps a file id to filename and .loc specifies the
source location of the succeeding instruction.

> How is 'file' supposed to be used? Would it make sense to
> include it in functions' linkage?

No, the id<->filename map is a global thing by design. I thought about this a
lot and I'm not sure how to improve it. I'm happy to change the command names
to something you find more suitable though.

Hi,

> > I'm not too keen on the syntax 'file' at toplevel does not
> > really look like debug. Same applies to the 'loc' operation.
> 
> The names are taken directly from gas, because the functionality is taken
> directly from gas too - .file maps a file id to filename and .loc specifies the
> source location of the succeeding instruction.
> 
> > How is 'file' supposed to be used? Would it make sense to
> > include it in functions' linkage?
> 
> No, the id<->filename map is a global thing by design. I thought about this a
> lot and I'm not sure how to improve it. I'm happy to change the command names
> to something you find more suitable though.

I've rebased bgs' patch on the current qbe master (f1b21d1) and worked in the
requested changes (mostly moving shared code from {amd,arm,rv}64 into shared
code in emit.c). I can send out a patch, but I'd like to cordinate with bgs on
that - they should get the credit.

@quentin: As bgs says, 'file' and 'loc' are just 1-1 gas directives. I can maybe
come up with some better names if you'd like.

-- Thomas

> diff --git a/amd64/isel.c b/amd64/isel.c
> index 5a64429..54aba26 100644
> --- a/amd64/isel.c
> +++ b/amd64/isel.c
> @@ -367,6 +367,7 @@ sel(Ins i, ANum *an, Fn *fn)
>  	case Oexts:
>  	case Otruncd:
>  	case Ocast:
> +	case Oloc:
>  	case_OExt:
>  Emit:
>  		emiti(i);

This emiti() call is immediately followed by calls to fixarg() which is avoided
in the other two implementations of sel(). At least this is true when based on
latest qbe master. Should the handling of Oloc here also avoid calling fixarg()?

-- Thomas