-
Notifications
You must be signed in to change notification settings - Fork 229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
compiler: Add fission-for-pressure subpass #2114
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2114 +/- ##
===========================================
- Coverage 87.77% 61.96% -25.81%
===========================================
Files 221 222 +1
Lines 39003 39107 +104
Branches 5067 5081 +14
===========================================
- Hits 34233 24232 -10001
- Misses 4208 14163 +9955
- Partials 562 712 +150
... and 129 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
continue | ||
|
||
# At this point we know we want to fission. But can we? | ||
for dep in c.scope.d_flow.independent(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not check this first and avoid all the retrieve/sets/...
ns = len(functions_shared) | ||
|
||
if not ns: | ||
ns = .001 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess safe to assume we won't have over 1k function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add some comment for this, even though not really necessary? Just to justify this option
ispace = c.ispace.lift(d) | ||
processed.append(c.rebuild(exprs=g1, ispace=ispace)) | ||
|
||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So at most one fission? Could check the remainder g
can be split too not sure of what would want
processed.append(c.rebuild(exprs=g0))
processed.append(fission_for_pressure(c.rebuild(exprs=g1, ispace=ispace)))
pass | ||
|
||
|
||
class IntelGoldenCode(Intel64): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GoldenCove?
ns = len(functions_shared) | ||
|
||
if not ns: | ||
ns = .001 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add some comment for this, even though not really necessary? Just to justify this option
t = grid.stepping_dim | ||
def test_nofission_as_unprofitable(self): | ||
""" | ||
Test there's no fission if not gonna increase number of collapsable loops. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"gonna" -> "going to"?
assert_structure(op, ['t,x,y', 't,x,y'], 't,x,y,y') | ||
def test_fission_partial(self): | ||
""" | ||
Test there's no fission if not gonna increase number of collapsable loops. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See previous comment
So far, this isn't enabled by default. More experimentation required
This PR also adds a new compiler flag to GNUCompiler when running on Skylake and later processors. I've tested it in a few places, and it seems to generally improve things. I'd like people to reproduce it. Note that you need to ensure DEVITO_PLATFORM is properly set to skx, clk, ... the autodetection mechanism, with the recent proliferation of intel architectures, doesn't seem to be as good as it was.