Skip to content

Add support for multiple single elements vld1 instructions #7531

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
+93 −8
Open

Add support for multiple single elements vld1 instructions #7531

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
43276e6
add vld1 multiple single elements support
icymanred Oct 24, 2025
c6b62ed
vld1
icymanred Oct 24, 2025
73cabe0
missing func
icymanred Oct 24, 2025
deb7f89
end macro
icymanred Oct 24, 2025
21a41de
Remove unnecessary break after return statement in arch/armv7/il.cpp
plafosse Oct 28, 2025
b1d3e22
Remove trailing semicolon arch/armv7/il.cpp
plafosse Oct 28, 2025
ab1b511
make ReadRegisterList accept a const reference arch/armv7/il.cpp
plafosse Oct 28, 2025
e7e53c8
add all data types for GetDataTypeSize function and default case,
icymanred Nov 1, 2025
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
20 changes: 14 additions & 6 deletions arch/armv7/armv7_disasm/armv7.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -910,15 +910,23 @@ typedef union _ieee754 {
float fvalue;
}ieee754;

typedef union _ieee754_double {
typedef union _ieee754_double
{
uint64_t value;
struct {
uint64_t fraction:52;
uint64_t exponent:11;
uint64_t sign:1;
struct
{
uint64_t fraction : 52;
uint64_t exponent : 11;
uint64_t sign : 1;
};
double fvalue;
}ieee754_double;
} ieee754_double;

struct DoubleWordRegisterList
{
uint8_t size;
uint8_t start;
};

#ifndef __cplusplus
typedef enum OperandClass OperandClass;
Expand Down
69 changes: 67 additions & 2 deletions arch/armv7/il.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,32 @@ static void ConditionExecute(LowLevelILFunction& il, Condition cond, ExprId true
il.AddInstruction(trueCase);
il.MarkLabel(falseCode);
}

// Returns an instructions datatype size in bits
static size_t GetDataTypeSize(DataType dt)
{
switch (dt)
{
case DT_I8:
case DT_8:
return 8;
break;
case DT_F16:
case DT_I16:
case DT_16:
return 16;
break;
case DT_F32:
case DT_I32:
case DT_32:
return 32;
break;
case DT_F64:
case DT_I64:
case DT_64:
return 64;
break;
}
};

static ExprId GetShifted(LowLevelILFunction& il, Register reg, uint32_t ShiftAmount, Shift shift)
{
Expand Down Expand Up @@ -159,7 +184,22 @@ static ExprId GetShifted(LowLevelILFunction& il, Register reg, uint32_t ShiftAmo
return 0;
}
}

static DoubleWordRegisterList ReadRegisterList(InstructionOperand instr) {
uint32_t val = instr.reg;
DoubleWordRegisterList dwrl;
#ifdef _MSC_VER
dwrl.size = __popcnt(val);
DWORD pos = 0;
_BitScanForward(&pos, val);
dwrl.start = pos;

#else
dwrl.size = __builtin_popcount(val);
dwrl.start = __builtin_ctz(val);

#endif
return dwrl;
}

static ExprId GetShiftedOffset(LowLevelILFunction& il, InstructionOperand& op)
{
Expand Down Expand Up @@ -5086,6 +5126,31 @@ bool GetLowLevelILForArmInstruction(Architecture* arch, uint64_t addr, LowLevelI
)
);
break;
case ARMV7_VLD1:
ConditionExecute(addrSize, instr.cond, instr, il, [&](size_t addrSize, Instruction& instr, LowLevelILFunction& il) {
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ConditionExecute call signature doesn't match the function definition shown in the context. The function expects (LowLevelILFunction& il, Condition cond, ExprId trueCase) but is being called with (addrSize, instr.cond, instr, il, lambda). This indicates either the wrong overload is being used or the function signature is incorrect.

Copilot uses AI. Check for mistakes.
Copy link
Author

@icymanred icymanred Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is an overload that takes in a lambda

switch (op1.cls)
{
case OperandClass::REG_LIST_DOUBLE:
DoubleWordRegisterList reglist = ReadRegisterList(op1);
Comment on lines +5140 to +5143
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The switch statement has no default case and no break statement after the REG_LIST_DOUBLE case. This will cause fall-through behavior and potential undefined behavior if other operand classes are encountered. Add a break statement after line 5146 and consider adding a default case.

Copilot uses AI. Check for mistakes.
uint32_t dregsize = get_register_size(REG_D0);
uint32_t regsize = get_register_size(op2.reg);
uint32_t dataSizeInBytes = GetDataTypeSize(instr.dataType) / 8;
for (unsigned int i = 0; i < reglist.size; i++)
{
uint32_t curOffset = i * dataSizeInBytes;
uint32_t curregind = REG_D0 + reglist.start + i;

il.AddInstruction(il.SetRegister(dregsize, curregind,
il.Load(dataSizeInBytes, il.Add(regsize, ILREG(op2), il.Const(regsize, curOffset)))));
}
if (op2.flags.wb)
{
il.AddInstruction(il.SetRegister(
regsize, op2.reg, il.Add(regsize, ILREG(op2), il.Const(regsize, dataSizeInBytes * reglist.size))));
}
}
});
break;
Comment on lines 5139 to 5165
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ConditionExecute call has incorrect parameters. Based on the function signature shown in the context, ConditionExecute takes (LowLevelILFunction& il, Condition cond, ExprId trueCase), but this call passes addrSize as the first parameter and uses a lambda that doesn't return an ExprId.

Suggested change
ConditionExecute(addrSize, instr.cond, instr, il, [&](size_t addrSize, Instruction& instr, LowLevelILFunction& il) {
switch (op1.cls)
{
case OperandClass::REG_LIST_DOUBLE:
DoubleWordRegisterList reglist = ReadRegisterList(op1);
uint32_t dregsize = get_register_size(REG_D0);
uint32_t regsize = get_register_size(op2.reg);
uint32_t dataSizeInBytes = GetDataTypeSize(instr.dataType) / 8;
for (unsigned int i = 0; i < reglist.size; i++)
{
uint32_t curOffset = i * dataSizeInBytes;
uint32_t curregind = REG_D0 + reglist.start + i;
il.AddInstruction(il.SetRegister(dregsize, curregind,
il.Load(dataSizeInBytes, il.Add(regsize, ILREG(op2), il.Const(regsize, curOffset)))));
}
if (op2.flags.wb)
{
il.AddInstruction(il.SetRegister(
regsize, op2.reg, il.Add(regsize, ILREG(op2), il.Const(regsize, dataSizeInBytes * reglist.size))));
}
}
});
break;
{
// Build the IL instructions for the VLD1 operation
switch (op1.cls)
{
case OperandClass::REG_LIST_DOUBLE:
{
DoubleWordRegisterList reglist = ReadRegisterList(op1);
uint32_t dregsize = get_register_size(REG_D0);
uint32_t regsize = get_register_size(op2.reg);
uint32_t dataSizeInBytes = GetDataTypeSize(instr.dataType) / 8;
for (unsigned int i = 0; i < reglist.size; i++)
{
uint32_t curOffset = i * dataSizeInBytes;
uint32_t curregind = REG_D0 + reglist.start + i;
il.AddInstruction(il.SetRegister(dregsize, curregind,
il.Load(dataSizeInBytes, il.Add(regsize, ILREG(op2), il.Const(regsize, curOffset)))));
}
if (op2.flags.wb)
{
il.AddInstruction(il.SetRegister(
regsize, op2.reg, il.Add(regsize, ILREG(op2), il.Const(regsize, dataSizeInBytes * reglist.size))));
}
break;
}
default:
break;
}
// Use il.Nop() as the trueCase since the instructions have already been added
ConditionExecute(il, instr.cond, il.Nop());
break;
}

Copilot uses AI. Check for mistakes.
Copy link
Member

@plafosse plafosse Oct 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two definitions of ConditionExecute consider this one

void ConditionExecute(size_t addrSize, Condition cond, Instruction& instr, LowLevelILFunction& il,
	std::function<void (size_t addrSize, Instruction& instr, LowLevelILFunction& il)> conditionalCode)
{

default:
//printf("Instruction: %s\n", get_operation(instr.operation));
ConditionExecute(il, instr.cond, il.Unimplemented());
Expand Down