Add support to allow bit-casting from f128 to i128 and then extracting 64 bits from the result.
|14192 ↗||(On Diff #161557)|
Nit: indentation is off.
|14195 ↗||(On Diff #161557)|
Perhaps for brevity and readability, combine this into the if:
if (SDValue CRTruncValue = DAGCombineTruncBoolExt(N, DCI)) return CRTruncValue;
|14203 ↗||(On Diff #161557)|
Nit: the more common way to declare references puts the ampersand immediately before the variable name.
|14209 ↗||(On Diff #161557)|
Why not just Op0.getOpcode() and Op0.getOperand(0)? Here as well as below.
|237 ↗||(On Diff #161557)|
I don't think it is OK to allow arbitrary integer types for the result nor arbitrary floating point types for operand 1. This really seems like it should use SDTCisVT. I realize that PPCbuild_fp128 has the same issue and we missed it, but I think we should favour fixing that one (in a separate patch) rather than making this one match.
|1 ↗||(On Diff #161557)|
I think once you add the BE portion to this patch, we should have another RUN to test that.
Instead of generating a special EXTRACT_FP128 node, could you bitcast to v2i64 and use EXTRACT_VECTOR_ELT instead? (I'm not that familiar with Power9, so it's possible that doesn't actually work for some reason.)
Removed the custom PPCISD node and it simplified the patch considerably.
Added conditions to handle bitconverts from f128 to a number of vector sizes (v2i64 down to v16i8).
Did not add the code for the bitconvert from the vector sizes to f128 because I have not been able to find an example to trigger the condition. It seems that the f128 is lowered into a store followed by a load.
|14331 ↗||(On Diff #170207)|
I think this can be further simplified and the duplicated code can be removed by essentially "looking through" the right shift and updating the element you need to extract.
I think the changes that are needed are clear enough that this doesn't require another review cycle. Approving this with the assumption that the required change will be made on the commit.
|14325 ↗||(On Diff #170442)|
This seems kind of misleading - makes the reader wonder what this "constant bit" is. I think something like // Switch the element number to extract. is a lot more descriptive.
|14329 ↗||(On Diff #170442)|
There needs to be an else here that will return SDValue() since it is not OK to keep going if we have an SRL with a non-constant shift amount or a shift amount other than 64.
// The right shift has to be by 64 bits. if (!ConstNode || ConstNode->getZextValue() != 64) return SDValue();
|14336 ↗||(On Diff #170442)|
The comment is redundant - basically just says that we're inside the if block.