-
Notifications
You must be signed in to change notification settings - Fork 10
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
Support absolute address for memory computations #274
Conversation
Please rebase it |
Updated. Thanks! |
@@ -431,6 +431,7 @@ static void simdOperandToArg(sljit_compiler* compiler, Operand* operand, JITArg& | |||
#include "FloatConvInl.h" | |||
#include "CallInl.h" | |||
#include "MemoryInl.h" | |||
#include "MemoryUtilInl.h" |
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.
It seems that MemoryUtilInl.h
and MemoryInl.h
files are used only in Backend.cpp
.
So, is this necessary to separate the origin file into two files?
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.
We plan to add atomic rmw to MemoryInl.h
because it depends on the address checking utility, and this file will grow quite big.
This change is not necessary, so I can revert this change if you prefer that.
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 see, it seems good :)
@@ -29,6 +29,7 @@ struct MemAddress { | |||
#endif /* SLJIT_32BIT_ARCHITECTURE */ | |||
#if defined(ENABLE_EXTENDED_FEATURES) | |||
CheckNaturalAlignment = 1 << 5, | |||
AbsoluteAddress = 1 << 6, |
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.
Is AbsoluteAddress
used only for extended features?
Would you please briefly introduce the role of this enum value ?
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 will add a comment. This option limits the result addressing form to a single base register with on offset. Will be also used by atomic rmw.
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 option limits the result addressing form to a single base register with on offset.
May I ask you more about this?
That is, why is this features applicable only for atomic operations?
I think that there are other possible cases that result addresses could be presented in a single register.
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.
Let me give you a background:
Normally, memory access can use all addressing modes of sljit such as:
[base_reg+offset], [absolute_offset], [base_reg+(offset_reg << shift)]
Hence, the address checker can return any of them without limitations.
However, (atomic) callbacks and atomic operations can only use [base_reg] addressing mode. This flag forces to produce only this type of addressing mode.
Since [base_reg] is a subset of [base_reg+offset] with offset 0, sometimes this addressing mode is returned without setting the 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.
Thanks for your kind explanation.
I'm not familiar with sljit system, so I need to learn about JITC as well.
Also adds minor code improvements Signed-off-by: Zoltan Herczeg [email protected]
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.
LGTM
The patch is based on #273