Hi,
On Fri, 20 Feb 2015, Edmund Grimley Evans wrote:
Does this look all right?
Do you have a testcase? I'll agree that there's possibly something fishy,
but the change doesn't look 100% right.
... looksy looksy ...
Hmm, maybe I trace your path how you found this :) When trying to use
VT_LLOCAL to replace VT_REF, when the member in question is not an array,
right? ;) Because then without your patch this example generates wrong
code under win64:
struct S {int a; int x[7];};
int f (int a, int b, int c, int d, struct S s)
{
return s.a;
}
So, something is wrong indeed. The problem I have with your patch:
@@ -4024,6 +4024,8 @@ ST_FUNC void unary(void)
vtop->type.t |= qualifiers;
/* an array is never an lvalue */
if (!(vtop->type.t & VT_ARRAY)) {
+ if ((vtop->r & (VT_VALMASK | VT_LVAL)) == (VT_LOCAL | VT_LVAL))
+ vtop->r += VT_LLOCAL - VT_LOCAL;
vtop->r |= lvalue_type(vtop->type.t);
#ifdef CONFIG_TCC_BCHECK
/* if bound checking, the referenced pointer must be checked */
is: this patches only one place where lvalue_type is called (i.e. where
something on the stack is lvalueized again). Why are the others trivially
correct and don't need such handling? Also, conceptually, I might have a
problem with this: it fixes up things after they went wrong. For
instance, why isn't the problem in gen_op? I mean, one invariant of
gen_op (when called with non-optimizable args) is that it leaves a
non-lvalue on the stack, so the code in unary() is correct as is, for
those cases. So, wouldn't it be better if the lvalue-to-rvalue conversion
would happen also with gen_op(+,0)?
Ciao,
Michael.
_______________________________________________
Tinycc-devel mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
