[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Tinycc-devel] [PATCH] deprecating VT_REF

From: Michael Matz
Subject: Re: [Tinycc-devel] [PATCH] deprecating VT_REF
Date: Sun, 22 Feb 2015 00:16:20 +0100 (CET)
User-agent: Alpine 2.00 (LNX 1167 2008-08-23)


On Sat, 21 Feb 2015, Edmund Grimley Evans wrote:

Michael Matz <address@hidden>:

I don't like this.  tccgen.c should become _less_ dependend on the
TARGET defines, not more.  Hence either VT_REF has a purpose and it
might make sense to use it in more backends, or it hasn't and should
be removed also from x86_64.

I agree, but inserting those #ifdefs is merely making the
architecture-specificity explicit, thereby perhaps encouraging someone
to do something about it. Without the #ifdefs, but with VT_REF only
used by the x86_64 back end, you have hidden target-specificity, which
is more dangerous.

I disagree about dangerous; it's only a problem for those targets, and presumably for them the generic handling of VT_REF is correct. If you mean dangerous as in "others might be tempted to use it in their target, even though the generic handling isn't correct for them", then, well, life is tough; it's not enough reason IMHO to uglify common code. Neither is trying to make others interested in removing the thing.

But apart from that philosophical argument, I claim that the concept of VT_REF is not target specific at all. It does have things in common with VT_LLOCAL, but is not _quite_ the same currently, as we now found out in the other mails. This not-quite-equality might indeed be bugs in how VT_LLOCAL is handled, e.g. VT_LLOCAL might be understood as an optimization to VT_REF (even though the latter was introduced later), the optimization being that lvalueness/referenceness can be changed by a simple bitflip, instead of generating code.

So, yes, ideally VT_LLOCAL and VT_REF should be merged. Though the question still is into which one. VT_LLOCAL is tricky, as the docu said :) And it currently contains bugs, when used for some things.

_If_ the issues with VT_LLOCAL can be fixed, then I'd be in favor of removing VT_REF. After such fixing goes in.

AFAIU VT_REF is added when an argument is passed in a caller
allocated buffer whose address itself is passed in a register (i.e.

Do you mean on the stack?

Usually, but not necessarily.  For a function like
  int f (int a; struct large b) { ... }
there are two ways to get to 'b':
1) it's saved on the stack by caller, the address will be right below
   the slot for 'a', or the first slot, if 'a' gets a register.  So
   addressof(b) will be something like sp+offset.
2) it's stored in some buffer (not necessarily stack) and the address of
   it is explicitely passed in the argument that would take the place of
   'b' were it of pointer type.  Now addressof(b) is not necessarily some
   offset from sp, but rather must be loaded (either from a register,
   or the stack slot that addressof value got)

Yes, VT_LLOCAL can nearly express the concept 2. VT_REF as well (just that it works right now).

On arm64 the caller sometimes allocates a buffer then passes a pointer
to that buffer on the stack, and I'm handling that with VT_LLOCAL,

Okay, that's the same as win64 then, which means you could have also used VT_REF. (I'm not saying you should have, just stating the fact.)

which is already widely used. (I checked that a lot of VT_LLOCALs are
generated when you run the test suite on i386, for example.)

Yes, VT_LLOCAL is used often. But it's used for only one thing currently (i.e. the number of concepts expressed by it is not very large): when an lvalue contained in a register is spilled, it becomes a stack slot with VT_LLOCAL. That makes sense, because "lvalue in a register" means the register contains the address of that lvalue (and then so does the stack slot). But for instance it's not totally clear what VT_LLOCAL means on something that never was a register (e.g. because it is not of right type). Also, is it ever okay to have VT_LLOCAL without VT_LVAL? (There should be nothing wrong about this, but does the code always handle this?)

I think perhaps it's time to try to figure out what the three flags (VT_LLOCAL, VT_LVAL and VT_LOCAL) really mean, which combinations make sense (under which circumstances) and which operations do what to the flags. If we had clear rules we probably can make VT_LLOCAL do exactly what's necessary in generic code, to make it safely and generally usable. Right now the whole VT_LLOCAL handling seems quite ad-hoc (and that results exactly in the omissions you saw with the '.' operator).

I'd definitely prefer it if someone could remove VT_REF altogether. Alternatively, someone could patch the documentation to explain exactly what VT_REF means, why it's needed, and how it differs from VT_LLOCAL. If neither of those patches is forthcoming I'd prefer something like my patch to be committed to just leaving it as it is.

Well, we're still discussing, so not all is lost yet :) I'd actually hope to have such description for VT_LLOCAL in the end, and remove VT_REF after the current threads are finished.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]