cc @shushanhf as this change also touches LA

Why change the order ?

In my opinion, the iiaHasInstrCount and iiaSetInstrCount is not the best at least within the currrent implementation.
I think the LoongArch64's implementation is better than the implementation of the ARM64 and X64.

There is no need to add iiaHasInstrCount and iiaSetInstrCount

The problem with iiaHasInstrCount is that the disassembler relies on it to check whether the branch is jumping to some label or the offset has been hardcoded in the codegen as a part of some prolog. If the iiaHasInstrCount is not used by the emitter then differentiating between those two cases becomes impossible

Moreover the current LA implementation utilizing iiaEncodedInstr has two major flaws. Firstly, it does not allow the iiaFieldHnd to be set (it takes it place in the union). Secondly, it very often activates two structs in the union itself which is Undefined Behavior by the cpp standard (only one member of the union can be active at the time). This works fine though because the compiler has the extension which allows to type pune trivial types using an union. However in this case the union is not being used to type pune thus this usage is in my opinion an overuse of a feature.

Lets consider this example:

    id->idAddr()->iiaSetInstrEncode(code);
    id->idAddr()->iiaLclVar.initLclVarAddr(varx, offs);

In those case two idAddrUnion member are activated: the iiaEncodedInstr and iiaLclVal. But they are not in the same anonymous struct therefore they activate different structs and so this is an UB.

In order to introduce the iiaFieldHnd field back I introduced yet another UB, this time by observing that iiaLclVal is not set in the context where the iiaFieldHnd would be useful. Because iiaFieldHnd occupied the same place in memory as iiaEncodedInstr I switched iiaLclVal and iiaEncodecInstr in theirs structs in order to circumvent this issue.

Because initially I didn't know that there are multiple active members at the same time I've tried to remove unused iiaJmpOffset from the Risc-V implementation but it ended in a crash. I've spend a LOT of time in order to find what was causing a problem. In my opinion the Risc-V implementation should move from the iiaEncodedInstr thus making it more similar with x86/arm64 implementations and getting rid off bugs that are very time consuming to find like this one.

The problem with iiaHasInstrCount is that the disassembler relies on it to check whether the branch is jumping to some label or the offset has been hardcoded in the codegen as a part of some prolog. If the iiaHasInstrCount is not used by the emitter then differentiating between those two cases becomes impossible

The iiaHasInstrCount and iiaSetInstrCount should be only used for ARM64/32 and x86/64.
For LA64, this is not needed. And the emit of LA64 is more efficiency than ARM64/32 and x86/64.

Moreover the current LA implementation utilizing iiaEncodedInstr has two major flaws. Firstly, it does not allow the iiaFieldHnd to be set (it takes it place in the union). Secondly, it very often activates two structs in the union itself which is Undefined Behavior by the cpp standard (only one member of the union can be active at the time). This works fine though because the compiler has the extension which allows to type pune trivial types using an union. However in this case the union is not being used to type pune thus this usage is in my opinion an overuse of a feature.

Lets consider this example:

    id->idAddr()->iiaSetInstrEncode(code);
    id->idAddr()->iiaLclVar.initLclVarAddr(varx, offs);

In those case two idAddrUnion member are activated: the iiaEncodedInstr and iiaLclVal. But they are not in the same anonymous struct therefore they activate different structs and so this is an UB.

In order to introduce the iiaFieldHnd field back I introduced yet another UB, this time by observing that iiaLclVal is not set in the context where the iiaFieldHnd would be useful. Because iiaFieldHnd occupied the same place in memory as iiaEncodedInstr I switched iiaLclVal and iiaEncodecInstr in theirs structs in order to circumvent this issue.

I will check this later.

Because initially I didn't know that there are multiple active members at the same time I've tried to remove unused iiaJmpOffset from the Risc-V implementation but it ended in a crash. I've spend a LOT of time in order to find what was causing a problem. In my opinion the Risc-V implementation should move from the iiaEncodedInstr thus making it more similar with x86/arm64 implementations and getting rid off bugs that are very time consuming to find like this one.

I don't agree to be more similar with x86/arm64.
In fact, I am now busying with the LoongArch64's Intrinsic which is reviewing and the Native-AOT which debuging, so I don't have enough time to refactor the whole emiter.
But in my plan, after I finish the LoongArch64's Intrinsic which is reviewing and the Native-AOT in 2024, I will refactor the whole emiter, of course implemented within some steps to optimize it.

If you are sure to make RISC-V more similar with x86/arm64 and don't wait the discusses within 2024Q3/Q4, please don't change the LA64's code and don't share with the LA64's code.

Sorry I didn't intend to break your code. The changes have been reverted for the LA architecture

You should not use the iiaHasInstrCount and iiaSetInstrCount which should be only used for ARM64/32 and x86/64 right now, and in fact these can also be optimized later

Could you please provide me with an information how to store an information about the jump origin? I thought the best idea would be to use the iiaHasInstrCount in order to memorize a non-label jump offset but I am open for any suggestions how to improve the current solution

Hi, @Bajtazar If you can give more info about your problem, maybe I can give you some advices.

some label or the offset has been hardcoded in the codegen as a part of some prolog.

which file and where the loacation ? What the disasmable format you want? by what tools you want this format?

Hi @shushanhf,
For example here the non-labeled branch is being emitted. I would like to capture an information about this in order to print in emitDispIns, for example bne t0, t1, pc+8 (+2 instructions) in the JitDisasm. Moreover, currently if the jump is not hardcoded the jump label is being printed. In the current PR I've done this via setting the iiaSetInstrCount and checking in the disassembly stage to determine which one of them is currently disassembled

Hi, @Bajtazar If you can give more info about your problem, maybe I can give you some advices.

some label or the offset has been hardcoded in the codegen as a part of some prolog.

which file and where the loacation ? What the disasmable format you want? by what tools you want this format?

And why you need this format printing ? by which testsing tools requires ?

If you can give me these info, I think I can give some suggestions to solve it tomorrow.

Hi, @Bajtazar If you can give more info about your problem, maybe I can give you some advices.

some label or the offset has been hardcoded in the codegen as a part of some prolog.

which file and where the loacation ? What the disasmable format you want? by what tools you want this format?

Hi @shushanhf, For example here the non-labeled branch is being emitted. I would like to capture an information about this in order to print in emitDispIns, for example bne t0, t1, pc+8 (+2 instructions) in the JitDisasm. Moreover, currently if the jump is not hardcoded the jump label is being printed. In the current PR I've done this via setting the iiaSetInstrCount and checking in the disassembly stage to determine which one of them is currently disassembled

oh, I know, this is very easy for LoongArch64 and RISC-V64, in fact the implementation of the LoongArch64 and RISC-V64 is more efficient than the ARM64/32 and X64/32.
But I have to push the suggestions tomorrow as off duty, is it OK tomorrow?

for example bne t0, t1, pc+8 (+2 instructions) in the JitDisasm.

BTW, why you need this format ? Is it for some testing tools required ?

@Bajtazar
Maybe you can reference the #96229 , I think there is no need to use iiaHasInstrCount and iiaSetInstrCount for RISCV64.