Page MenuHomePhabricator

[FPEnv] Added a special UnrollVectorOp method to deal with the chain on StrictFP opcodes

Authored by ajwock on May 28 2019, 1:03 PM.



This change creates UnrollVectorOp_StrictFP. The purpose of this is to address a failure that consistently occurs when calling StrictFP functions on vectors whose number of elements is 3 + 2n on most platforms, such as PowerPC or SystemZ. The old UnrollVectorOp method does not expect that the vector that it will unroll will have a chain, so it has an assert that prevents it from running if this is the case. This new StrictFP version of the method deals with the chain while unrolling the vector. With this new function in place during vector widending, llc can run vector-constrained-fp-intrinsics.ll for SystemZ successfully.

Diff Detail

Event Timeline

ajwock created this revision.May 28 2019, 1:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 28 2019, 1:03 PM
ajwock updated this revision to Diff 202040.May 29 2019, 1:19 PM

Modified the SystemZ test for this fix so that it also works for SystemZ Z13, which is afflicted by a different errror involving function return types at an earlier stage in compilation. To circumvent this, all v3f64 tests were changed so that they took a pointer parameter, modified the value at that pointer, and returned void. These still test StrictFP code generation while avoiding an unrelated platform specific error.

1341 ↗(On Diff #202040)

Missing a space in i = 0.

1343 ↗(On Diff #202040)

What's the purpose of e here? Why not:

for (unsigned j = 1, j != N->getNumOperands(); ++j) {

Is it just saving an accessor call?

ajwock updated this revision to Diff 202171.May 30 2019, 6:15 AM
ajwock marked 2 inline comments as done.

Added space (i= 0 -> i = 0)

ajwock added inline comments.May 30 2019, 6:16 AM
1341 ↗(On Diff #202040)

Updating the diff to include the space.

1343 ↗(On Diff #202040)

Yes, this is just some strength reduction. The looping structure was borrowed from SelectionDAG::UnrollVectorOp.

ajwock marked an inline comment as not done.May 30 2019, 6:17 AM
cameron.mcinally accepted this revision.May 30 2019, 8:43 AM

LGTM, with one nit inline...

1320 ↗(On Diff #202171)

Not sure about LLVM's coding style, but is this extra line ok?

This revision is now accepted and ready to land.May 30 2019, 8:43 AM
This revision was automatically updated to reflect the committed changes.