Api Audit and Fixing Up Effect Checks #191

gilbo · 2022-05-11T15:21:38Z

This PR will collect all of my work to make sure the new effect checks are working.

This PR will not yet try to deprecate or strip out the old effect system.

This PR will also not yet try to replace the old bounds check with the new effect-checking machinery. (Another PR should replace the bounds checking / assert checking, and then yet another PR can strip out the old effect checking stuff entirely)

codecov · 2022-05-11T20:22:44Z

Codecov Report

Merging #191 (4a53990) into master (4264975) will decrease coverage by 2.10%.
The diff coverage is 83.50%.

❗ Current head 4a53990 differs from pull request most recent head 15815b4. Consider uploading reports for the commit 15815b4 to get more accurate results

@@            Coverage Diff             @@
##           master     #191      +/-   ##
==========================================
- Coverage   84.72%   82.61%   -2.11%     
==========================================
  Files          66       66              
  Lines       14633    14906     +273     
==========================================
- Hits        12398    12315      -83     
- Misses       2235     2591     +356

Impacted Files	Coverage Δ
src/exo/pyparser.py	`73.45% <ø> (-0.04%)`	⬇️
src/exo/API.py	`71.60% <60.71%> (-0.87%)`	⬇️
src/exo/LoopIR_scheduling.py	`83.53% <78.82%> (-0.44%)`	⬇️
src/exo/new_eff.py	`89.58% <95.45%> (+1.22%)`	⬆️
src/exo/new_analysis_core.py	`80.43% <100.00%> (+0.17%)`	⬆️
src/exo/parse_fragment.py	`67.09% <100.00%> (-29.66%)`	⬇️
src/exo/platforms/gemmini.py	`94.17% <100.00%> (+0.03%)`	⬆️
tests/gemmini/conv/test_gemmini_conv_ae.py	`100.00% <100.00%> (ø)`
tests/test_schedules.py	`100.00% <100.00%> (ø)`
tests/amx/test_amx_instr.py	`11.19% <0.00%> (-88.81%)`	⬇️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4264975...15815b4. Read the comment docs.

…arsing

src/exo/new_eff.py

yamaguchi1024 · 2022-05-16T21:46:25Z

src/exo/LoopIR_scheduling.py

@@ -767,103 +768,102 @@ def __init__(self, proc, stmt):
        self.orig_proc = proc
        self.win_stmt  = stmt

+        print(proc)


Is this for debugging?

yes, I assume so; it's gone now.

yamaguchi1024 · 2022-05-16T21:47:19Z

src/exo/pyparser.py

@@ -393,8 +393,6 @@ def parse_num_type(self, node, is_arg = False):
                    if isinstance(node.slice, pyast.Tuple):
                        dims = node.slice.elts
                    else:
-                        assert isinstance(node.slice,


Why did you delete this assert?

I believe I relaxed the constraint that dependent sizes are simple (i.e. only a name or constant). It seems we already have all the infrastructure to support expressions in tensor types but never removed this check. If you're concerned this might break something, maybe we can add some tests to increase our confidence?

…o bounds checking more generally

…v code

…y checks for aliasing and config variables as procedure arguments, both of which could lead to unsound analyses, and the latter of which was causing problems for fixing the loop fissioning checks

yamaguchi1024 · 2022-07-30T01:29:13Z

@gilbo Should this be closed then? Are these commits contained in the merged PR?

gilbo · 2022-07-30T06:33:42Z

No. I need to work on this next. It’s another massive PR to fix the underlying analysis

…

On Jul 29, 2022, at 6:29 PM, Yuka Ikarashi ***@***.***> wrote: @gilbo Should this be closed then? Are these commits contained in the merged PR? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

tests/golden/test_schedules/test_expand_dim5.txt

yamaguchi1024 · 2022-11-03T21:40:55Z

src/exo/platforms/gemmini.py

@@ -942,7 +946,7 @@ def do_st_acc_i8(
 st_acc_i8_s2_v2 = st_acc_i8_s2_v2.reorder_stmts('src_tmp = _', 'ConfigStore.scale = _')
 st_acc_i8_s2_v2 = st_acc_i8_s2_v2.reorder_stmts('src_tmp : _', 'ConfigStore.scale = _')
 st_acc_i8_s2_v2 = st_acc_i8_s2_v2.fission_after('ConfigStore.scale = _', n_lifts=2)
-st_acc_i8_s2_v2 = st_acc_i8_s2_v2.configwrite_after('ConfigStore.scale = _', ConfigStore, 'dst_stride', 'stride(dst, 2)')
+st_acc_i8_s2_v2 = st_acc_i8_s2_v2.configwrite_after('ConfigStore.scale = _', ConfigStore, 'dst_stride', 'stride(dst, 1)')


What's happening??

We should create a stride typecheck test

yamaguchi1024 · 2022-11-04T20:34:47Z

src/exo/LoopIR_scheduling.py

-                                  "an allocation might be buried "
-                                  "in a different scope than some use-site")
+        pass
+        #pre_allocs  = { s.name for s in pre if isinstance(s,LoopIR.Alloc) }


Why is this commented out?

yamaguchi1024 · 2022-11-04T20:45:33Z

src/exo/LoopIR_scheduling.py


 # --------------------------------------------------------------------------- #
 # --------------------------------------------------------------------------- #
 # The Passes to export

 class Schedules:
+    DoRenameBuf = _DoRenameBuf
+    DoAssignNewVarBefore = _DoAssignNewVarBefore
+    DoDeleteDeadCode = _DoDeleteDeadCode


We should go through the motivation of these primitives together and also write tests

This PR is partially merging #191, which was authored by Gilbert * merge partition_loop * merge small assert in parse_fragment * merge lift_if changes * merge FV * merge lift_alloc_simple * Update src/exo/LoopIR_scheduling.py Co-authored-by: Alex Reinking <[email protected]> * Update src/exo/LoopIR_scheduling.py Co-authored-by: Alex Reinking <[email protected]> * Update src/exo/LoopIR_scheduling.py Co-authored-by: Alex Reinking <[email protected]> * Update src/exo/LoopIR_scheduling.py Co-authored-by: Alex Reinking <[email protected]> Co-authored-by: Alex Reinking <[email protected]>

SamirDroubi · 2023-01-20T00:18:03Z

src/exo/parse_fragment.py

@@ -174,6 +174,9 @@ def __init__(self, pat, proc, stmt, scope):
        else:
            self.srcinfo = stmt.srcinfo

+            assert scope == "before", ("Non-before scope for parsing "


Change merged in #277

SamirDroubi · 2023-01-20T23:47:30Z

tests/test_schedules.py

+def test_lift_alloc_simple3(golden):
+    @proc
+    def bar(n: size, A: i8[n]):
+        for k in seq(0, n):
+            for i in seq(0, n):
+                for j in seq(0, n):
+                    tmp_a: i8
+                    tmp_a = A[i]
+
+    bar = bar.lift_alloc_simple('tmp_a : _', n_lifts=3)
+    assert str(bar) == golden
+
+def test_lift_alloc_simple_fv_error():
+    @proc
+    def bar(n: size, A: i8[n]):
+        for k in seq(0, n):
+            for i in seq(0, n):
+                for j in seq(0, n):
+                    tmp_a: i8[k + 1]
+                    tmp_a[k] = A[i]
+
+    with pytest.raises(SchedulingError,
+            match='Cannot lift allocation statement'):
+        bar = bar.lift_alloc_simple('tmp_a : _', n_lifts=3)
+
+


Changed merged in #277

SamirDroubi · 2023-01-20T23:48:14Z

tests/golden/test_schedules/test_lift_alloc_simple3.txt

+def bar(n: size, A: i8[n] @ DRAM):
+    tmp_a: i8 @ DRAM
+    for k in seq(0, n):
+        for i in seq(0, n):
+            for j in seq(0, n):
+                tmp_a = A[i]


Change merged in #277

SamirDroubi · 2023-01-20T23:56:26Z

src/exo/LoopIR_scheduling.py

    def __init__(self, proc, loop_stmt, num):
        self.stmt         = loop_stmt
        self.partition_by = num
-        self.second       = False
-        self.second_iter  = None

        super().__init__(proc)



All changes to _PartitionLoop were merged in #277

SamirDroubi · 2023-01-20T23:57:39Z

src/exo/LoopIR_scheduling.py

+        assert isinstance(expr, (LoopIR.Read, LoopIR.StrideExpr,
+                                 LoopIR.Const))


Changes merged in #277

SamirDroubi · 2023-01-21T00:04:02Z

src/exo/LoopIR_scheduling.py

-                raise SchedulingError("specified lift level {self.n_lifts} "+
-                                      "is higher than the number of loop "+
-                                      "{len(self.ctrl_ctxt)}")
+                raise SchedulingError(f"specified lift level {self.n_lifts} "
+                                      f"is more than {len(self.ctrl_ctxt)}, "
+                                      f"the number of loops "
+                                      f"and ifs above the allocation")
+            if len(s.type.shape()) > 0:
+                szvars  = set.union(*[ _FV(sz) for sz in s.type.shape() ])
+                for i in self.get_ctrl_iters():
+                    if i in szvars:
+                        raise SchedulingError(
+                            f"Cannot lift allocation statement {s} past loop "
+                            f"with iteration variable {i} because "
+                            f"the allocation size depends on {i}.")


Changed merged in #277

SamirDroubi · 2023-01-24T03:09:26Z

tests/test_schedules.py

@@ -625,10 +801,10 @@ def bar(n: size, src: [R][n], dst: [R][n]):
                dst[i] = src[i] + src[i + 1]

    @proc
-    def foo(x: R[50, 2]):


Changes applied in #307

SamirDroubi · 2023-01-24T03:11:41Z

src/exo/LoopIR_unification.py

@@ -48,6 +49,7 @@ def __init__(self, proc, subproc, stmt_block):
        self.live_vars      = ChainMap()

        super().__init__(proc)
+        Check_Aliasing(self.proc)


Changes were applied in #307

SamirDroubi · 2023-01-24T03:15:52Z

src/exo/LoopIR_scheduling.py

@@ -1192,13 +1264,13 @@ def __init__(self, proc, if_stmt, n_lifts):
        assert is_pos_int(n_lifts)

        self.target = if_stmt
-        self.loop_deps = vars_in_expr(if_stmt.cond)


_DoLiftIf is deprecated now

SamirDroubi · 2023-01-24T03:17:39Z

src/exo/LoopIR_scheduling.py


+# remove when _DoDoubleFission is removed


Cannot be deprecated yet, still used in _FissionLoops

SamirDroubi · 2023-01-24T03:33:38Z

tests/golden/test_schedules/test_unify4.txt

@@ -1,2 +1,2 @@
-def foo(x: R[50, 2] @ DRAM):


Changes were applied in #307

* Changes to `_InlineWindow` * Changes to `_CallSwap` * `Check_Aliasing` in `_BindConfig` and `_BindExpr` * Changes to `_DoExpandDim` * Changes to `_DoRearrangeDim` * Changes to `_DoRemoveLoop` * Changes to `_DoFissionAfterSimple` * Changes to `_DoFuseIf` * Added `fuse_loop` tests * Changes to `_DoAddLoop` * Changes to `_DoExtractMethod` * Changes to `_AssertIf` * Changes to `_DoDataReuse` * Changes to `_DoStageWindow` * Changes to `_DoBoundAlloc` * Changes to `DoReplace` * Update commented line in `_DoAddUnsafeGuard` * fixed AMX test error * Update src/exo/LoopIR_scheduling.py --------- Co-authored-by: Kevin Qian <[email protected]> Co-authored-by: Yuka Ikarashi <[email protected]>

gilbo added 8 commits May 9, 2022 18:09

more refactorings

55a5cbd

added check and fixed bug for data reuse

11e92db

Merge branch 'master' into api-refactor

b582aee

Merge branch 'api-refactor' into api-audit-and-fix

ede0ad8

fix merged code

d4a8a1b

fixed inline window test 'golden' so it fails, the correct behavior

7ad73ad

fixed up inline window transformation

0642b43

fixed some bugs that were exposed after fixing inline window

d160bc5

gilbo added 2 commits May 12, 2022 00:17

added fuse loop tests; relaxed deprecated constraint on tensor type p…

4c1fa29

…arsing

added appropriate checks to fuse-if scheduling primitive

caec0ed

yamaguchi1024 reviewed May 16, 2022

View reviewed changes

src/exo/new_eff.py Show resolved Hide resolved

yamaguchi1024 reviewed May 16, 2022

View reviewed changes

gilbo added 13 commits May 18, 2022 16:45

audited add and remove loop, and also assert if

838e939

fixed up implementation of partition loop

2cd3542

fixed up implementation of partition loop

2440e1b

refactored bound_alloc, but not yet fully checked by SMT. Deferring t…

aedf044

…o bounds checking more generally

fixed up rearrange_dim

3e53b43

Merge branch 'master' into api-audit-and-fix

1862107

removed weird scoping convention for 'expand_dim'

00e322b

added three directives and showed how to simplify initial GEMMINI con…

e4b776a

…v code

resolved merge conflicts

4e8e11c

fixed up checking for expand_dims

45da448

fixed up various issues in expand_dim and lift_alloc

dd97004

Merge branch 'master' into api-audit-and-fix

4a53990

halfway through adding loop_fission checks; this commit puts in safet…

15815b4

…y checks for aliasing and config variables as procedure arguments, both of which could lead to unsound analyses, and the latter of which was causing problems for fixing the loop fissioning checks

getting local code checked in to merge

97afb84

yamaguchi1024 reviewed Nov 2, 2022

View reviewed changes

tests/golden/test_schedules/test_expand_dim5.txt Show resolved Hide resolved

yamaguchi1024 marked this pull request as ready for review November 3, 2022 16:30

yamaguchi1024 reviewed Nov 3, 2022

View reviewed changes

yamaguchi1024 reviewed Nov 4, 2022

View reviewed changes

yamaguchi1024 mentioned this pull request Nov 4, 2022

First attempt to merge #191 #277

Merged

yamaguchi1024 reviewed Nov 4, 2022

View reviewed changes

yamaguchi1024 marked this pull request as draft November 4, 2022 23:36

yamaguchi1024 mentioned this pull request Nov 20, 2022

Effects merge attempt 2 #293

Merged

SamirDroubi reviewed Jan 20, 2023

View reviewed changes

SamirDroubi reviewed Jan 24, 2023

View reviewed changes

yamaguchi1024 closed this Jun 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Api Audit and Fixing Up Effect Checks #191

Api Audit and Fixing Up Effect Checks #191

gilbo commented May 11, 2022

codecov bot commented May 11, 2022 •

edited

Loading

yamaguchi1024 May 16, 2022

gilbo May 25, 2022

yamaguchi1024 May 16, 2022

gilbo May 25, 2022

yamaguchi1024 commented Jul 30, 2022

gilbo commented Jul 30, 2022 via email

yamaguchi1024 Nov 3, 2022

yamaguchi1024 Nov 4, 2022

yamaguchi1024 Nov 4, 2022

yamaguchi1024 Nov 4, 2022

SamirDroubi Jan 20, 2023

SamirDroubi Jan 20, 2023

SamirDroubi Jan 20, 2023

SamirDroubi Jan 20, 2023

SamirDroubi Jan 20, 2023

SamirDroubi Jan 21, 2023

SamirDroubi Jan 24, 2023

SamirDroubi Jan 24, 2023

SamirDroubi Jan 24, 2023

SamirDroubi Jan 24, 2023

SamirDroubi Jan 24, 2023

		assert isinstance(expr, (LoopIR.Read, LoopIR.StrideExpr,
		LoopIR.Const))

Api Audit and Fixing Up Effect Checks #191

Api Audit and Fixing Up Effect Checks #191

Conversation

gilbo commented May 11, 2022

codecov bot commented May 11, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yamaguchi1024 commented Jul 30, 2022

gilbo commented Jul 30, 2022 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 11, 2022 •

edited

Loading