https://bugs.kde.org/show_bug.cgi?id=362329
Carl Love <c...@us.ibm.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #98666|0 |1
is obsolete| |
--- Comment #6 from Carl Love <c...@us.ibm.com> ---
Created attachment 99056
--> https://bugs.kde.org/attachment.cgi?id=99056&action=edit
updated, 0001-Power-PC-Add-support-for-ISA-3.0-part-3.patch
I like your idea for combining create_DCM_32() and create_DCM() so we don't
have lots of similarly named functions. But, I am not too keen on passing a
list of IROp's to the function as it isn't really obvious from the function
name why it would need them. My preference would be to just pass the size of
the expr to be created and then just setup the IROp's in the function. I think
it is cleaner to the reader and a bit less error prone. Let the function make
sure it generates 64 bit or 32 bit iops rather then the user messing up on one
of the arguments. Also eliminates the need to verify the IROp's passed are all
compatible with each other.
I wasn't aware of the the issue with the back end generating code multiple
times. So basically the rule of thumb should be: If you use a computed value
multiple times, compute it into a temp not an expression.
With the above two things in mind, I rewrote create_DCM() as follows:
static IRExpr * create_DCM ( IRType size, IRTemp NaN, IRTemp inf, IRTemp zero,
IRTemp dnorm, IRTemp pos)
{
/* This is a general function for creating the DCM for a 32-bit or
64-bit expression based on the passes size.
*/
IRTemp neg;
IROp opAND, opOR, opSHL;
vassert( ( size == Ity_I32 ) || ( size == Ity_I64 ) );
if ( size == Ity_I32 ) {
opSHL = Iop_Shl32;
opAND = Iop_And32;
opOR = Iop_Or32;
neg = newTemp( Ity_I32 );
} else {
opSHL = Iop_Shl64;
opAND = Iop_And64;
opOR = Iop_Or64;
neg = newTemp( Ity_I64 );
}
assign( neg, unop( Iop_1Uto64, mkNOT1( unop( Iop_64to1,
mkexpr ( pos ) ) ) ) );
return binop( opOR,
binop( opSHL, mkexpr( NaN ), mkU8( 6 ) ),
binop( opOR,
binop( opOR,
binop( opOR,
binop( opSHL,
binop( opAND,
mkexpr( pos ),
mkexpr( inf ) ),
mkU8( 5 ) ),
binop( opSHL,
binop( opAND,
mkexpr( neg ),
mkexpr( inf ) ),
mkU8( 4 ) ) ),
binop( opOR,
binop( opSHL,
binop( opAND,
mkexpr( pos ),
mkexpr( zero ) ),
mkU8( 3 ) ),
binop( opSHL,
binop( opAND,
mkexpr( neg ),
mkexpr( zero ) ),
mkU8( 2 ) ) ) ),
binop( opOR,
binop( opSHL,
binop( opAND,
mkexpr( pos ),
mkexpr( dnorm ) ),
mkU8( 1 ) ),
binop( opAND,
mkexpr( neg ),
mkexpr( dnorm ) ) ) ) );
}
I chose to just pass the values in as temps and not mess around with changing
them in the function. It seemed cleaner.
Does the function look reasonable?
I did something very similar to combine functions is_Denorm_32() and
is_Denorm(). I updated Quad_precision_gt() to use temps not expressions when
the computed value was used more then once. I also updated is_Zero_V128 () as
suggested.
--
You are receiving this mail because:
You are watching all bug changes.
