Repository navigation
Conversation
github-actions
Bot
added
cranelift
| ;; APX: as with `ineg` above, prefer the NDD form of `not` for 64-bit values | ||
| ;; when APX is enabled so that the input register is not clobbered. | ||
| (rule bnot_apx_ndd 1 (lower (bnot $I64 x)) | ||
| (if-let true (use_apx)) | ||
| (x64_notq_vm x)) | ||
|
|
||
| ;; APX: the same for `i32` and narrower; see `iadd_apx_ndd_32` for why `i8` and | ||
| ;; `i16` use the 32-bit form. | ||
| (rule bnot_apx_ndd_32 1 (lower (bnot $I32 x)) | ||
| (if-let true (use_apx)) | ||
| (x64_notl_vm x)) | ||
| (rule bnot_apx_ndd_16 1 (lower (bnot $I16 x)) | ||
| (if-let true (use_apx)) | ||
| (x64_notl_vm (gpr_to_gpr_mem_32 (put_in_gpr x)))) | ||
| (rule bnot_apx_ndd_8 1 (lower (bnot $I8 x)) | ||
| (if-let true (use_apx)) | ||
| (x64_notl_vm (gpr_to_gpr_mem_32 (put_in_gpr x)))) |
There was a problem hiding this comment.
Would it make sense to put these rules within the x64_not helper perhaps where applicable? That way other usages of x64_not would also benefit from these new lowering rules as well. This'd also apply to most of the other rules changed in this PR too.
| (rule iadd_apx_ndd_16 1 (lower (iadd $I16 x y)) | ||
| (if-let true (use_apx)) | ||
| (x64_addl_rvm (put_in_gpr x) (gpr_to_gpr_mem_32 (put_in_gpr y)))) | ||
| (rule iadd_apx_ndd_8 1 (lower (iadd $I8 x y)) | ||
| (if-let true (use_apx)) | ||
| (x64_addl_rvm (put_in_gpr x) (gpr_to_gpr_mem_32 (put_in_gpr y)))) |
There was a problem hiding this comment.
I might have missed this from earlier discussion, but why do these rules explicitly use put_in_gpr for all operands? Do APX instructions not support 8/16-bit memory load operands?
| pretty_print_hexadecimal(&assemble(&inst.into())), | ||
| "62F47C18F7D2" | ||
| ); | ||
| } |
There was a problem hiding this comment.
Personally I'd say it's probably fine to drop all of these tests. They're pretty verbose and I don't think it's worth adding individual tests for all instructions. The "oracles" here are opaque hex strings which can't really be reviewed or verified, so I think we'll be best to lean on other testing methods like differential fuzzing (should we enable that for a disassembler that supports apx) or CI-based runtest execution (e.g. QEMU with apx emulation or intel's emulator).
Actually, along those lines, we already have some tests executed through the Intel SDE, for example this job. Does the SDE support APX? If so can you add directives in CLIF runtests with test x86_64 has_apx to get full execution testing for the new instructions? That's probably the best possible testing we can hope for since our disasemblers don't support APX (or it's at least not wired up)
Second of a series splitting #14696
into one PR per instruction class, following #14930.
Adds the APX NDD (new data destination) forms of the unary ops neg and not, which
use a VM encoding where the written operand takes the vvvv slot and ModRM.reg still
holds the opcode extension, plus the 32-bit RVM forms of add, sub, and, or and xor.
i8 and i16 lower through the 32-bit form, since they are already widened. has_apx is off by default, so nothing changes unless it's
enabled. More NDD patches to follow.