Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Align inner loops #44370
Align inner loops #44370
Changes from 58 commits
32236ac
a2ec5d1
89b5812
a84b8be
a51759a
2f57c78
b4848f8
fe92c62
51c3171
0b227a8
a9c9764
7f1f787
aac18a4
5c3c40e
fb91d41
0b8eb78
072a113
7e1328a
d49e84d
f6b5135
256aae5
26c3c84
6bb1a75
03f6ba6
8036061
809fc85
5b9b7a0
5584d71
f66ee24
f2f935d
99ea31f
8f64963
1c85c3c
ef0b149
599ad43
97fd373
37b0cdb
c0cc8af
26f7e61
305b812
f8bdfec
a205cc0
d576e9c
28480d1
bf03842
dae9749
a153742
ef02fbb
e7e0d68
4b0e64d
6fddce8
bad5685
8c30a96
f32f560
23177b0
9f3cb2d
74620ed
b100226
dbeb7d6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This condition seems odd to me: Presumably, you've already determined that an "isLoopAlign" block is the top of a loop. But now, any block that jumps to that is considered a back edge. What if the lexically first block of the loop is the target of branches that aren't loop branches: forward branches jumping to the loop top, or non-loop backward branches? (The non-loop backward branches might not matter since presumably your algorithm will hit an in-loop backward branch first, but possibly marking them could confuse other code? E.g., what if the first block in a non-first loop branched back to the top of the first loop? I'm not sure if all these cases can occur, due to loop canonicalization, etc., but this case seems not too specific)
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.
emitSetLoopBackEdge()
checks theigNum
and accordingly decides if the edge is a back-edge or not.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.
Looks like
opts.compJitAlignLoopBoundary
doesn' get set to anything in release if we are not doing adaptive alignment.And whatever value it does get set to should be overridable in debug, in case you want to experiment.
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 missed setting
opts.compJitAlignLoopBoundary
for non-adaptive / release case. I will do it. Are you also suggesting to give a way to setcompJitAlignLoopBoundary
even for adaptive? Currently, the logic I have for adaptive is restricted to have a singlealign
instruction of max 15 bytes. I will have to think and restructure some code to make that alignment boundary flexible for adaptive scenario. If you agree, I can do that in a separate PR?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.
Sure, it can wait.
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 realize this is preexisting code, but this seems like a roundabout way to check if we are prejitting; why not just check the prejit flag?
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.
You mean use
JIT_FLAG_PREJIT
instead ofJIT_FLAG_RELOC
?