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
Types from beginning to end #37
Types from beginning to end #37
Changes from 11 commits
0d41365
fe4610d
2f4d462
9a013f3
5831094
055931c
08f8d36
2578acc
89337ae
9fa9485
3457d53
b5b3183
8a94037
61f28a8
98ae71e
f06972a
aa23eae
c9be95c
61d343d
1ad6dbf
d82cd3d
9b30942
20e42a0
3ad2392
e7d77a7
b4af7c9
8013c43
a836f0b
9d23bd2
55e1ec0
e339100
6c4570d
a94f836
933ed59
312d4e7
ac386a8
ab4eac2
4423fee
c57b388
ab7dcf8
db2d432
1bd10b3
66f7289
b125c17
bee8136
afebae2
e669db3
c9ccbb2
6615957
9b71982
dc49514
4e3e0f8
ec7b772
974bdff
32f6a44
b2d090d
b14fa1c
8281a44
ec35e50
6574a1d
e830a9f
339e07c
84f1e10
80749cf
81aaf8e
a400f57
4a70c4c
cd2381a
f922e21
13b8210
33c195c
49c9970
e594ee5
b95d879
b7be438
49f6762
6c49cac
7253339
7b84f66
771b9b4
df8ffb0
5a60f48
cc09b8b
836f99b
acd725c
f3fab2d
a8241fa
6c027c7
c7f87aa
88715b4
1104d93
b222c7f
6411d36
40b4430
f1f876c
8282edc
3a9c115
79c22b1
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.
To expound on this: If we get here, we should assume that we have a case like e.g.
a := b
whereb
is an array type, and we’re trying to emitb
, in which case we just emit a load and be done w/ it.If we’re dealing w/ something like
b[0]
instead (ora
ina := b
, or&b
), then we should check for that case when codegenning the subscript operator (or the assignment or addressof operator) and not even try and emitb
in the first place iff it is an array. This is also how cases like@foo := bar
are handled because dereferencing suffers from all the same problems...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 works fine in simple cases (e.g.
&b
),b
has to be an lvalue anyway for us to be able to take its address, and we don’t need to emit a load to it to know its address.Where this probably doesn’t work atm is if we have something like
(if cond { a } else { b }) := c
, but then again, I’m not sure this case would work currently, even ifa
andb
are regular variables... so perhaps we might have to change it to where it only actually loads the value once we know that is used by some other instruction (and used AS A VALUE, not as an address to e.g. store to). I’m not sure though if this should happen by somehow not generating a load when we only want the lvalue or by removing the load afterwards if it’s clear that it’s unused.Optimisations would take care of the latter anyway, so perhaps a solution would be to make optimising dead loads mandatory? Only trivially dead loads, i.e. loads that have no users at all, that is. We wouldn’t have to account for loads that are indirectly dead because, although they are used to compute some value, that value is also dead code. That doesn’t matter in this case since if we were to emit a load when we actually just need an lvalue, then the load proper would be completely unused.
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.
The top-down approach (i.e. checking if a value needs to be loaded or if we just want its address) would entail emitting potentially arbitrarily complex and nested expressions while not actually loading anything and instead just collecting the lvalues. All of this is just an idea tho; I haven’t thought too much about this in a while and it also feels like I’m overcomplicating things here candidly...
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.
Honestly, I feel really dumb when I think about this problem. It seems so simple, but it's really hard, lmao. One thing I am confused about is how do I actually emit a load of a pointer (i.e. the pointer) without dereferencing it?
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 entire thing is basically more or less related to getting something like this to work properly:
I guess there is a finite number of expressions that may return an lvalue (e.g.
if
expressions may, butwhile
loops can’t), so one option would be to leavecodegen_expr
untouched and add a separatecodegen_lvalue
or sth that gets called e.g. when emitting an assignment or array subscript, and instead of loading the value, it just ‘loads’ the address. For example, the value of the if expression in(if a { @b ⟩ else { @c }) := 5
at runtime would be the address ofb
orc
, to which we could then emit an (indirect) store. This entire thing is basically just about handling lvalues properly. Once we can codegen e.g. that if expression, then arrays should just work because they’re basically the same thing...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'm beginning to understand somewhat now; a major point of confusion was the phrasing "not emit a load" or similar (I'll be the first to admit that is on me). I was confused as to where the value would come from if it's never loaded. But it seems like what you are referencing is the fact that the "load" has already been emitted, and therefore it's not necessary to emit it again. Instead, we don't emit a load because one already exists; we can just use that instructions value. What helped me see this was you pointing out the current handling of dereference, as I know how that works and why it's needed.
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.
Can
be turned into
Not saying it should, really just wondering if they are equivalent
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.
They’re not really the same. The latter is what you’d get if you did this:
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.
An alloca is a variable declaration
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.
The codegen for the
if
is everything after and including thebr.cond
. It’s assumed thatb
andc
are defined earlier on in the function; I included thealloca
s mostly to make clear that these are supposed to be variables