Skip to content

Commit

Permalink
previously:
Browse files Browse the repository at this point in the history
 First_readable_addr = 4K * 8 on most targets
 First_readable_addr = 8K * 8 on sparc
 could be but isn't 8k on alpha/ia64

now:
 4K on all targets -- not 4K * 8, but just 4K
 m3front does the multiplication by 8 and doing
 it twice made the value too high.
 Could be but isn't 8K on ia64/alpha/sparc. It'd
 be a small optimization in unusual code, and
 isn't worth the targets varying.

Test case is p263.

Still maybe want to eliminate it, but at least
all targets are now the same here.
  • Loading branch information
jaykrell committed Aug 26, 2015
1 parent e1df15b commit ca69e1d
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 27 deletions.
35 changes: 33 additions & 2 deletions m3-sys/m3middle/src/Target.i3
Expand Up @@ -464,11 +464,42 @@ VAR (*CONST*)
word boundaries. *)

(* NIL checking *)
First_readable_addr: CARDINAL;
CONST First_readable_addr: CARDINAL = 4096 (* * Char.size *);
(* Read or write references to addresses in the range [0..First_readable-1]
will cause an address faults. Hence, no explicit NIL checks are needed
for dereferencing with offsets in this range. *)
for dereferencing with offsets in this range.
m3front only checks the size of surrounding accessed type,
i.e. the field or array the element is within. This is overly conservative.
Historically this was off by 8. Historically we tried to use the more
target specific page size, like 8K on SPARC (and could be on Alpha
and IA64 also). But in the name of removing target-specificity, just 4K always.
Additional comments on the matter:
The effect of First_readable_addr is that (static?) array indices
(offsets) lower than it (and positive?) do not have a NULL check on the
array base. Reading NULL + an offset less than First_readable_addr is
assumed to access violate the same as reading NULL. It is a nice
optimization. Setting the value too low results in correct but
suboptimal code. However just setting it to a small non-zero number
should provide most of the benefit. Setting the value too high results
in missing NULL checks -- a loss of safety enforcement. Typically
setting it to one hardware page is a good estimate, since if NULL is
not accessible, nor is any address on the same page. As well, setting
it to getpagesize, whatever the granularity of mmap/VirtualAlloc, often
larger than a hardware page, is another good guess.
Historically this value was off by a factor of 8.
Historically we used 8K for Sparc. Probably should for IA64 and Alpha.
But now we use 4K for all, which is ok.
As well, it is not about static array references currently.
It is about the size of the containing type, even if accessing
a small offset.
*)

VAR (*CONST*)
(* floating point values *)
All_floats_legal : BOOLEAN;
(* If all bit patterns are "legal" floating point values (i.e. they can
Expand Down
28 changes: 3 additions & 25 deletions m3-sys/m3middle/src/Target.m3
Expand Up @@ -41,11 +41,10 @@ PROCEDURE IsAMD64(): BOOLEAN =
RETURN TextUtils.StartsWith(System_name, "AMD64_");
END IsAMD64;

PROCEDURE IsSPARC(): BOOLEAN =
<*UNUSED*>PROCEDURE IsSPARC(): BOOLEAN =
CONST startsWith = TextUtils.StartsWith;
BEGIN
RETURN (TextUtils.StartsWith(System_name, "S")
AND (TextUtils.StartsWith(System_name, "SPARC")
OR TextUtils.StartsWith(System_name, "SOL")));
RETURN startsWith(System_name, "SPARC") OR startsWith(System_name, "SOL");
END IsSPARC;

PROCEDURE Init (system: TEXT; in_OS_name: TEXT; backend_mode: M3BackendMode_t): BOOLEAN =
Expand Down Expand Up @@ -120,21 +119,6 @@ PROCEDURE Init (system: TEXT; in_OS_name: TEXT; backend_mode: M3BackendMode_t):

Aligned_procedures := TRUE;

(* The effect of First_readable_addr is that (static?) array indices
(offsets) lower than it (and positive?) do not have a NULL check on the
array base. Reading NULL + an offset less than First_readable_addr is
assumed to access violate the same as reading NULL. It is a nice
optimization. Setting the value too low results in correct but
suboptimal code. However just setting it to a small non-zero number
should provide most of the benefit. Setting the value too high results
in missing NULL checks -- a loss of safety enforcement. Typically
setting it to one hardware page is a good estimate, since if NULL is
not accessible, nor is any address on the same page. As well, setting
it to getpagesize, whatever the granularity of mmap/VirtualAlloc, often
larger than a hardware page, is another good guess. *)

First_readable_addr := 4096 * Char.size;

(* add the system-specific customization *)

(* 64bit *)
Expand Down Expand Up @@ -169,12 +153,6 @@ PROCEDURE Init (system: TEXT; in_OS_name: TEXT; backend_mode: M3BackendMode_t):
Little_endian := FALSE;
END;

(* SPARC: 8K pages *)

IF IsSPARC() THEN
First_readable_addr := 8192 * Char.size;
END;

(* x86 and AMD64 allow unaligned loads/stores *)

IF IsX86() OR IsAMD64() THEN
Expand Down

0 comments on commit ca69e1d

Please sign in to comment.