Skip to content
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

i#6698 instr_t ISA mode: move from flags to its own field #6699

Merged
merged 22 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c4b9371
i#6698 instr_t ISA mode: move from flags to its own field
edeiana Mar 8, 2024
0e9fee7
Testing if having isa_mode as a single byte solves failing tests.
edeiana Mar 10, 2024
95d438c
Manually fixed clang-format issue.
edeiana Mar 10, 2024
f00c3ae
Reintroducing previous behavior for instr_get_isa_mode().
edeiana Mar 10, 2024
a9dd260
Removed commented code.
edeiana Mar 10, 2024
9053352
Testing if increase of instr_t size is responsible for failing tests.
edeiana Mar 10, 2024
5b05926
Reverting isa_mode back to byte size.
edeiana Mar 10, 2024
4966abf
Added encode_api.h to all core/ir/ARCH/instr.c since it defines dr_is…
edeiana Mar 10, 2024
457abcb
Merge branch 'master' into i6698-instr-isa-mode
edeiana Mar 10, 2024
f6c1806
Added set_isa_mode() to instr_create() for AARCH64 and RISCV64.
edeiana Mar 11, 2024
05fd2a4
Addressed PR comments.
edeiana Mar 11, 2024
3e3b984
Fixed formatting.
edeiana Mar 11, 2024
a907b96
Added issue # to XXX.
edeiana Mar 11, 2024
70fa7c9
Doxygen comment to regular comment.
edeiana Mar 11, 2024
5f0ce16
All archs except x86_64 set instr_t isa_mode from dcontext.
edeiana Mar 11, 2024
2acf41a
Fixes formatting.
edeiana Mar 11, 2024
f0c9a5d
ARM and future arches set instr_t isa_mode from global dcontext.
edeiana Mar 11, 2024
8a58add
Testing if setting instr_t isa_mode using the global dcontext works
edeiana Mar 11, 2024
696861b
ifdef for x86 seems necessary.
edeiana Mar 12, 2024
4d16f39
CLIENT_ASSERT condition was flipped. Fixed it.
edeiana Mar 12, 2024
6794108
Removed unnecessary header.
edeiana Mar 12, 2024
d91989c
Merge branch 'master' into i6698-instr-isa-mode
edeiana Mar 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions core/ir/aarch64/instr.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,21 @@
#include "../globals.h"
#include "instr.h"
edeiana marked this conversation as resolved.
Show resolved Hide resolved
#include "decode.h"

#include "encode_api.h"
edeiana marked this conversation as resolved.
Show resolved Hide resolved
#include "opcode_names.h"

/* XXX: currently only A64 is supported for instruction encoding.
edeiana marked this conversation as resolved.
Show resolved Hide resolved
* We want to add support for A64 decoding and synthetic ISA encoding as well.
* XXX i#1684: move this function to core/ir/instr_shared.c once we can support
* all architectures in the same build of DR.
*/
bool
instr_set_isa_mode(instr_t *instr, dr_isa_mode_t mode)
{
return (mode == DR_ISA_ARM_A64);
}

dr_isa_mode_t
instr_get_isa_mode(instr_t *instr)
{
return DR_ISA_ARM_A64;
if (mode != DR_ISA_ARM_A64)
return false;
edeiana marked this conversation as resolved.
Show resolved Hide resolved
instr->isa_mode = DR_ISA_ARM_A64;
return true;
}

int
Expand Down
21 changes: 9 additions & 12 deletions core/ir/arm/instr.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,28 +32,25 @@

#include "../globals.h"
#include "instr.h"
#include "encode_api.h"
#include "decode.h"

/* FIXME i#1551: add A64 and Thumb support throughout */
edeiana marked this conversation as resolved.
Show resolved Hide resolved

/* XXX: currently only A64 and Thumb is supported for instruction encoding.
edeiana marked this conversation as resolved.
Show resolved Hide resolved
* We want to add support for A64 and Thumb decoding and synthetic ISA encoding as well.
* XXX i#1684: move this function to core/ir/instr_shared.c once we can support
* all architectures in the same build of DR.
*/
bool
instr_set_isa_mode(instr_t *instr, dr_isa_mode_t mode)
{
if (mode == DR_ISA_ARM_THUMB)
instr->flags |= INSTR_THUMB_MODE;
else if (mode == DR_ISA_ARM_A32)
instr->flags &= ~INSTR_THUMB_MODE;
else
if ((mode != DR_ISA_ARM_THUMB) && (mode != DR_ISA_ARM_A32)) {
edeiana marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
instr->isa_mode = mode;
return true;
}

dr_isa_mode_t
instr_get_isa_mode(instr_t *instr)
{
return TEST(INSTR_THUMB_MODE, instr->flags) ? DR_ISA_ARM_THUMB : DR_ISA_ARM_A32;
}

int
instr_length_arch(dcontext_t *dcontext, instr_t *instr)
{
Expand Down
13 changes: 1 addition & 12 deletions core/ir/instr.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,19 +202,8 @@ enum {
INSTR_DO_NOT_EMIT = 0x10000000,
/* PR 251479: re-relativization support: is instr->rip_rel_pos valid? */
INSTR_RIP_REL_VALID = 0x20000000,
#ifdef X86
/* PR 278329: each instr stores its own mode */
INSTR_X86_MODE = 0x40000000,
#elif defined(ARM)
/* We assume we don't need to distinguish A64 from A32 as you cannot swap
* between them in user mode. Thus we only need one flag.
* XXX: we might want more power for drdecode, though the global isa_mode
* should be sufficient there.
*/
INSTR_THUMB_MODE = 0x40000000,
#endif
/* PR 267260: distinguish our own mangling from client-added instrs */
INSTR_OUR_MANGLING = 0x80000000,
INSTR_OUR_MANGLING = 0x40000000,
edeiana marked this conversation as resolved.
Show resolved Hide resolved
};

#define DR_TUPLE_TYPE_BITS 4
Expand Down
7 changes: 7 additions & 0 deletions core/ir/instr_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,13 @@ struct _instr_t {
byte num_dsts;
byte num_srcs;

/** This field assumes values of type #dr_isa_mode_t.
edeiana marked this conversation as resolved.
Show resolved Hide resolved
edeiana marked this conversation as resolved.
Show resolved Hide resolved
*/
/* Instruction ISA mode to support multiple architectures in the same build of DR
* (xref i#6698 i#1684).
*/
edeiana marked this conversation as resolved.
Show resolved Hide resolved
byte isa_mode;

union {
struct {
/* for efficiency everyone has a 1st src opnd, since we often just
Expand Down
12 changes: 11 additions & 1 deletion core/ir/instr_shared.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ instr_create(void *drcontext)
instr_set_isa_mode(instr, X64_CACHE_MODE_DC(dcontext) ? DR_ISA_AMD64 : DR_ISA_IA32);
#elif defined(ARM)
instr_set_isa_mode(instr, dr_get_isa_mode(dcontext));
#elif defined(AARCH64)
instr_set_isa_mode(instr, DR_ISA_ARM_A64);
#elif defined(RISCV64)
instr_set_isa_mode(instr, DR_ISA_RV64IMAFDC);
edeiana marked this conversation as resolved.
Show resolved Hide resolved
#endif
return instr;
}
Expand Down Expand Up @@ -442,6 +446,12 @@ private_instr_encode(dcontext_t *dcontext, instr_t *instr, bool always_cache)
return len;
}

dr_isa_mode_t
instr_get_isa_mode(instr_t *instr)
{
return (dr_isa_mode_t)instr->isa_mode;
}

#define inlined_instr_get_opcode(instr) \
(IF_DEBUG_(CLIENT_ASSERT(sizeof(*instr) == sizeof(instr_t), "invalid type"))( \
((instr)->opcode == OP_UNDECODED) \
Expand Down Expand Up @@ -4196,4 +4206,4 @@ move_mm_avx512_reg_opcode(bool aligned64)
}

#endif /* !STANDALONE_DECODER */
/****************************************************************************/
/****************************************************************************/
edeiana marked this conversation as resolved.
Show resolved Hide resolved
17 changes: 10 additions & 7 deletions core/ir/riscv64/instr.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,20 @@

#include "../globals.h"
#include "instr.h"
#include "encode_api.h"

/* XXX: currently only RISCV64 is supported for instruction encoding.
* We want to add support for RISCV64 decoding and synthetic ISA encoding as well.
* XXX i#1684: move this function to core/ir/instr_shared.c once we can support
* all architectures in the same build of DR.
*/
bool
instr_set_isa_mode(instr_t *instr, dr_isa_mode_t mode)
{
return (mode == DR_ISA_RV64IMAFDC);
}

dr_isa_mode_t
instr_get_isa_mode(instr_t *instr)
{
return DR_ISA_RV64IMAFDC;
if (mode != DR_ISA_RV64IMAFDC)
return false;
edeiana marked this conversation as resolved.
Show resolved Hide resolved
instr->isa_mode = DR_ISA_RV64IMAFDC;
return true;
}

int
Expand Down
29 changes: 11 additions & 18 deletions core/ir/x86/instr.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "instr.h"
#include "decode.h"
#include "decode_private.h"
#include "encode_api.h"
edeiana marked this conversation as resolved.
Show resolved Hide resolved
#include "instr_create_shared.h"

#ifdef X64
Expand All @@ -51,9 +52,9 @@ void
instr_set_x86_mode(instr_t *instr, bool x86)
edeiana marked this conversation as resolved.
Show resolved Hide resolved
{
if (x86)
instr->flags |= INSTR_X86_MODE;
instr->isa_mode = DR_ISA_IA32;
else
instr->flags &= ~INSTR_X86_MODE;
instr->isa_mode = DR_ISA_AMD64;
}

/*
Expand All @@ -63,37 +64,29 @@ instr_set_x86_mode(instr_t *instr, bool x86)
bool
instr_get_x86_mode(instr_t *instr)
edeiana marked this conversation as resolved.
Show resolved Hide resolved
{
return TEST(INSTR_X86_MODE, instr->flags);
return instr->isa_mode == DR_ISA_IA32;
}
#endif

/* XXX: currently only x86 and x64 are supported for instruction encoding.
* We want to add support for x86 and x64 decoding and synthetic ISA encoding as well.
* XXX i#1684: move this function to core/ir/instr_shared.c once we can support
* all architectures in the same build of DR.
*/
bool
instr_set_isa_mode(instr_t *instr, dr_isa_mode_t mode)
{
#ifdef X64
if (mode == DR_ISA_IA32)
instr_set_x86_mode(instr, true);
else if (mode == DR_ISA_AMD64)
instr_set_x86_mode(instr, false);
else
if (mode != DR_ISA_IA32 && mode != DR_ISA_AMD64)
return false;
#else
if (mode != DR_ISA_IA32)
return false;
#endif
instr->isa_mode = mode;
return true;
}

dr_isa_mode_t
instr_get_isa_mode(instr_t *instr)
{
#ifdef X64
return TEST(INSTR_X86_MODE, instr->flags) ? DR_ISA_IA32 : DR_ISA_AMD64;
#else
return DR_ISA_IA32;
#endif
}

int
instr_length_arch(dcontext_t *dcontext, instr_t *instr)
{
Expand Down
Loading