Skip to content
This repository has been archived by the owner. It is now read-only.

fold: keep type of emitted CONV in sync with its mode #53

Merged
merged 1 commit into from Nov 26, 2019

Conversation

mraleph
Copy link

@mraleph mraleph commented Nov 25, 2019

Alternative fix for #37

When emitting CONV make sure that its type matches its destination IRType.

This keeps IR fully internally consistent with respect to types - i.e. if
we push narrowing CONV Dt.St upwards through an arithmetic operation of type
St we end up with arithmetic operation of type Dt and two convertions
CONV Dt.St which narrow the operands.

Previous variantion of the fix introduced slight inconsistency with types
(inserted convertions were CONV int.St while arithmetic operation was still of
type Dt).

Alternative fix for LuaJIT#37

When emitting CONV make sure that its type matches its destination IRType.

This keeps IR fully internally consistent with respect to types - i.e. if
we push narrowing CONV Dt.St upwards through an arithmetic operation of type
St we end up with arithmetic operation of type Dt and two convertions
CONV Dt.St which narrow the operands.

Previous variantion of the fix introduced slight inconsistency with types
(inserted convertions were CONV int.St while arithmetic operation was still of
type Dt).

Signed-off-by: Vyacheslav Egorov <vegorov@google.com>
@mraleph
Copy link
Author

mraleph commented Nov 25, 2019

I have tried creating a regression test but so far I have not been successful (we need to have a combination of factors - arithmetic operation needs to be folded away and then a widening conversion needs to be applied to the result of the narrowing conversion). I will think a bit more about it.

@mraleph
Copy link
Author

mraleph commented Nov 25, 2019

/cc @siddhesh

@siddhesh siddhesh merged commit 4dba12d into moonjit:master Nov 26, 2019
@siddhesh
Copy link
Collaborator

Thanks @mraleph !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants