PPC 64 the ISA 3.0 darn (deliver a random number) instruction is not supported
Created attachment 136990 [details] patch to add darn instruction support The attached patch to add support for the darn instruction uses a clean helper to add the support. The clean helper uses an inline assembly statement to call the darn (Deliver a Random Number) instruction. This is not the normal or preferred way to support an instruction. AMD 64 used a dirty helper to support its random number generator. I looked at C code random number generators but did not a good solution. Julian, please let me know if you are ok with this non-standard implementation for supporting the random number generation instruction.
(In reply to Carl Love from comment #1) Handing it off to the hardware via a helper function is the right thing to do. But you can't use a clean helper for this; that breaks the rules for clean helpers. A clean helper * must produce a value which depends only on its arguments * given the same argument, always produces the same result. and the IR optimiser relies (or may rely) on the above being true. You need to use a dirty helper. Also I'm not happy about the replacement C implementations; I'm not sure when they would ever get used. I'd prefer if they were removed.
Argh, I inadvertently added the alternate C code solution. That was for playing. I thought I had removed that. I will change to a dirty helper and get rid of the C code solution. Thanks
Created attachment 136999 [details] patch to add darn instruction support Updated the patch. - Removed the C code random number generator code. Was not supposed to be included in previous patch. - Reworked the helper to make it a dirty helper that returns the random value. Patch tested on Power 8, Power 9 and on ISA 3.1 prototype hardware. Please let me know if the updated patch is good to go. Thanks.
(In reply to Carl Love from comment #4) > Created attachment 136999 [details] > patch to add darn instruction support +ULong darn_dirty_helper ( UInt L ) +{ + ... +# else + val = 0xFFFFFFFFFFFFFFFFULL; /* error */ +# endif Given that you initialise `val` to the error value outside of any ifdef, this #else clause is redundant. You could remove it. +static Int dis_darn ( UInt prefix, UInt theInstr, + const VexAbiInfo* vbi ) .. + if (L == 3) + /* Hardware reports illegal instruction if L = 3. */ + vpanic("ERROR: darn instruction L=3 is not valid\n"); Generate SIGILL for an unrecognised instruction, don't assert. Please always do this -- I've asked multiple times before.
Created attachment 137204 [details] patch to add darn instruction support Updated patch Fixed: Return false on L=3 to generate SIGILL on illegal instruction. Removed #else clause in dirty helper as the return value, val, is already set to the error value.
(In reply to Carl Love from comment #6) > Created attachment 137204 [details] > patch to add darn instruction support +static Int dis_darn ( UInt prefix, UInt theInstr, + + IRExpr** args = mkIRExprVec_1( mkU32( L ) ); + + d = unsafeIRDirty_1_N ( + rD, + 0/*regparms*/, + "darn_dirty_helper", + fnptr_to_fnentry( vbi, &darn_dirty_helper ), + args ); This wants a comment saying that there are no effects on memory or guest state, since it is unusual for a dirty helper to have no such effects. Otherwise looks fine to me.
Created attachment 138144 [details] patch to add darn instruction support Updated the patch. Added a comment to thedis_darn() function in guest_ppc_toIR.c and darn_dirty_helper in guest_ppc_helpers stating the dirty helper does not change the guest state or guest memory. It simply returns the value from running the darn instruction on the host.
Patch committed. commit 8afb49abe04a341d60b441c1f09a956aeccf0bbb (HEAD -> master, origin/master, origin/HEAD) Author: Carl Love <cel@us.ibm.com> Date: Mon Mar 22 17:55:05 2021 -0500 PPC64: Add support for the darn instruction
Closing issue