Port of RAII's writable/addressable/c_lval revamp

Author: lwaern-intel

py/dml/codegen.py

@@ -1163,7 +1163,7 @@ def expr_unop(tree, location, scope):
     elif op == 'post--':  return mkPostDec(tree.site, rh)
     elif op == 'sizeof':
         if (compat.dml12_misc not in dml.globals.enabled_compat
-            and not isinstance(rh, ctree.LValue)):
+            and not rh.addressable):
             raise ERVAL(rh.site, 'sizeof')
         return codegen_sizeof(tree.site, rh)
     elif op == 'defined': return mkBoolConstant(tree.site, True)
@@ -1525,7 +1525,7 @@ def eval_type(asttype, site, location, scope, extern=False, typename=None,
                     etype = expr.node_type
                 else:
                     raise expr.exc()
-            elif (not isinstance(expr, ctree.LValue)
+            elif (not expr.addressable
                   and compat.dml12_misc not in dml.globals.enabled_compat):
                 raise ERVAL(expr.site, 'typeof')
             else:

py/dml/ctree.py

@@ -1187,21 +1187,12 @@ def truncate_int_bits(value, signed, bits=64):
         return value & mask
 
 class LValue(Expression):
-    "Somewhere to read or write data"
+    """An expression whose C representation is always an LValue, whose address
+    is always safe to take, in the sense that the duration that address
+    remains valid is intuitively predictable by the user"""
     writable = True
-
-    def write(self, source):
-        rt = realtype(self.ctype())
-        if isinstance(rt, TEndianInt):
-            return (f'{rt.dmllib_fun("copy")}(&{self.read()},'
-                    + f' {source.read()})')
-        return '%s = %s' % (self.read(), source.read())
-
-    @property
-    def is_stack_allocated(self):
-        '''Returns true only if it's known that writing to the lvalue will
-           write to stack-allocated data'''
-        return False
+    addressable = True
+    c_lval = True
 
 class IfExpr(Expression):
     priority = 30
@@ -2628,13 +2619,13 @@ def make_simple(cls, site, rh):
                                TPtr(TVoid())],
                               TVoid())))
         if (compat.dml12_misc not in dml.globals.enabled_compat
-            and not isinstance(rh, LValue)):
+            and not rh.addressable):
             raise ERVAL(rh.site, '&')
         return AddressOf(site, rh)
 
     @property
     def is_pointer_to_stack_allocation(self):
-        return isinstance(self.rh, LValue) and self.rh.is_stack_allocated
+        return self.rh.is_stack_allocated
 
 def mkAddressOf(site, rh):
     if dml.globals.compat_dml12_int(site):
@@ -2672,7 +2663,8 @@ def is_stack_allocated(self):
 
     @property
     def is_pointer_to_stack_allocation(self):
-        return isinstance(self.type, TArray) and self.is_stack_allocated
+        return (isinstance(safe_realtype_shallow(self.type), TArray)
+                and self.is_stack_allocated)
 
 mkDereference = Dereference.make
 
@@ -2794,7 +2786,7 @@ def mkUnaryPlus(site, rh):
         rh, _ = promote_integer(rh, rhtype)
     else:
         raise ICE(site, "Unexpected arith argument to unary +")
-    if isinstance(rh, LValue):
+    if rh.addressable or rh.writable:
         # +x is a rvalue
         rh = mkRValue(rh)
     return rh
@@ -2820,7 +2812,7 @@ def make_simple(cls, site, rh):
         rhtype = safe_realtype(rh.ctype())
         if not isinstance(rhtype, (IntegerType, TPtr)):
             raise EINCTYPE(site, cls.op)
-        if not isinstance(rh, LValue):
+        if not rh.addressable:
             if isinstance(rh, BitSlice):
                 hint = 'try %s= 1' % (cls.base_op[0],)
             else:
@@ -4586,11 +4578,13 @@ def read(self):
 
 mkStaticVariable = StaticVariable
 
-class StructMember(LValue):
+class StructMember(Expression):
     priority = 160
     explicit_type = True
     @auto_init
     def __init__(self, site, expr, sub, type, op):
+        # Write of StructMembers rely on them being C lvalues
+        assert not expr.writable or expr.c_lval
         assert_type(site, expr, Expression)
         assert_type(site, sub, str)
 
@@ -4607,11 +4601,12 @@ def read(self):
 
     @property
     def is_stack_allocated(self):
-        return isinstance(self.expr, LValue) and self.expr.is_stack_allocated
+        return self.expr.is_stack_allocated
 
     @property
     def is_pointer_to_stack_allocation(self):
-        return isinstance(self.type, TArray) and self.is_stack_allocated
+        return (isinstance(safe_realtype_shallow(self.type), TArray)
+                and self.is_stack_allocated)
 
 def try_resolve_len(site, lh):
     if isinstance(lh, NonValue):
@@ -4750,18 +4745,28 @@ def is_stack_allocated(self):
 
     @property
     def is_pointer_to_stack_allocation(self):
-        return isinstance(self.type, TArray) and self.is_stack_allocated
+        return (isinstance(safe_realtype_shallow(self.type), TArray)
+                and self.is_stack_allocated)
 
-class VectorRef(LValue):
+class VectorRef(Expression):
     slots = ('type',)
     @auto_init
     def __init__(self, site, expr, idx):
+        assert not expr.writable or expr.c_lval
         self.type = realtype(self.expr.ctype()).base
     def read(self):
         return 'VGET(%s, %s)' % (self.expr.read(), self.idx.read())
-    def write(self, source):
-        return "VSET(%s, %s, %s)" % (self.expr.read(), self.idx.read(),
-                                     source.read())
+    # No need for write, VGET results in an lvalue
+
+    @property
+    def writable(self):
+        return self.expr.writable
+    @property
+    def addressable(self):
+        return self.expr.addressable
+    @property
+    def c_lval(self):
+        return self.expr.c_lval
 
 def mkIndex(site, expr, idx):
     if isinstance(idx, NonValue):
@@ -4835,7 +4840,7 @@ def read(self):
 
     @property
     def is_pointer_to_stack_allocation(self):
-        return (isinstance(self.type, TPtr)
+        return (isinstance(safe_realtype_shallow(self.type), TPtr)
                 and self.expr.is_pointer_to_stack_allocation)
 
 def mkCast(site, expr, new_type):
@@ -5008,12 +5013,17 @@ def explicit_type(self):
     def type(self):
         assert self.explicit_type
         return self.expr.type
+    # Since addressable and readable are False this may only ever be leveraged
+    # by DMLC for optimization purposes
+    @property
+    def c_lval(self):
+        return self.expr.c_lval
     @property
     def is_pointer_to_stack_allocation(self):
         return self.expr.is_pointer_to_stack_allocation
 
 def mkRValue(expr):
-    if isinstance(expr, LValue) or expr.writable:
+    if expr.addressable or expr.writable:
         return RValue(expr.site, expr)
     return expr
 

py/dml/expr.py

@@ -109,11 +109,19 @@ class Expression(Code):
     # bitslicing.
     explicit_type = False
 
-    # Can the expression be assigned to?
-    # If writable is True, there is a method write() which returns a C
-    # expression to make the assignment.
+    # Can the expression be safely assigned to in DML?
+    # This implies write() can be safely used.
     writable = False
 
+    # Can the address of the expression be taken safely in DML?
+    # This implies c_lval, and typically implies writable.
+    addressable = False
+
+    # Is the C representation of the expression an lvalue?
+    # If True, then the default implementation of write() must not be
+    # overridden.
+    c_lval = False
+
     def __init__(self, site):
         assert not site or isinstance(site, Site)
         self.site = site
@@ -139,10 +147,16 @@ def apply(self, inits, location, scope):
         'Apply this expression as a function'
         return mkApplyInits(self.site, self, inits, location, scope)
 
+    @property
+    def is_stack_allocated(self):
+        '''Returns true only if it's known that the storage for the value that
+           this expression evaluates to is temporary to a method scope'''
+        return False
+
     @property
     def is_pointer_to_stack_allocation(self):
         '''Returns True only if it's known that the expression is a pointer
-           to stack-allocated data'''
+           to storage that is temporary to a method scope'''
         return False
 
     def incref(self):
@@ -156,6 +170,17 @@ def copy(self, site):
         return type(self)(
             site, *(getattr(self, name) for name in self.init_args[2:]))
 
+    # Return a (principally) void-typed C expression that write a source to the
+    # storage this expression represents
+    # This should only be called if either writable or c_lval is True
+    def write(self, source):
+        assert self.c_lval
+        rt = realtype(self.ctype())
+        if isinstance(rt, TEndianInt):
+            return (f'{rt.dmllib_fun("copy")}(&{self.read()},'
+                    + f' {source.read()})')
+        return '%s = %s' % (self.read(), source.read())
+
 class NonValue(Expression):
     '''An expression that is not really a value, but which may validly
     appear as a subexpression of certain expressions.
@@ -202,11 +227,14 @@ def __str__(self):
         return self.str or self.cexpr
     def read(self):
         return self.cexpr
-    def write(self, source):
-        assert self.writable
-        return "%s = %s" % (self.cexpr, source.read())
     @property
     def writable(self):
+        return self.c_lval
+    @property
+    def addressable(self):
+        return self.c_lval
+    @property
+    def c_lval(self):
         return self.type is not None
 
 mkLit = Lit