This is an archive of the discontinued LLVM Phabricator instance.

[FPEnv] Split double width StrictFP vector operations as needed
ClosedPublic

Authored by cameron.mcinally on Jul 1 2018, 7:52 AM.

Details

Summary

Here's a little patch to split illegal StrictFP vector operations. It's very much like the code already committed to expand StrictFP vector operations.

Some notes:

  1. There's one line that I'm not too happy about. I'll add a comment inline to elucidate it.
  1. The double width vector math lib calls and rounding calls are not split on X86, but rather expanded. The newly added tests for those may be overkill. Thoughts on this?
  1. It may make sense to break out the StrictFP FMA operations into their own test file. We're testing some of these non-FMA operations twice for NO-FMA and HAS-FMA. That said, the generated assembly is slightly different when FMAs are enabled, so there may be some value in keeping them as-is.

Diff Detail

Repository
rL LLVM

Event Timeline

There's one line that I'm not too happy about. I'll add a comment inline to elucidate it.

Ah, I can't seem to add a comment inline. The line in question is lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1082...

EVT ValueVTs[] = {OpsLo[1].getValueType(), MVT::Other};

Querying the split vector type this way, i.e. OpsLo[1].getValueType(), seems a little magic-numberish. I was contemplating using VectorType's getHalfElementsVectorType(...), but I do not see it used anywhere else in master.

Any suggestions on how to proceed?

craig.topper added inline comments.Jul 1 2018, 10:48 PM
lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
1082 ↗(On Diff #153650)

You can use DAG.GetSplitDestVTs(N->getValueType(0)) to get the high and lo VTs. That's the correct way to do this.

craig.topper added inline comments.Jul 1 2018, 10:55 PM
lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
1082 ↗(On Diff #153650)

Though if we don't want to assume the VT of operand 1 matches the destination VT, then its also wrong to call GetSplitVector on the input operands. You would need to call getTypeAction on that VT and ensure that it's also TypeSplitVector and if its not call DAG.SplitVector instead.

Updating the diff based on Craig's input...

cameron.mcinally marked 2 inline comments as done.Jul 2 2018, 8:47 AM
cameron.mcinally added inline comments.
lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
1082 ↗(On Diff #153650)

Thanks, Craig. That was what I was looking for.

Good catch on the input vector types that aren't the same as the result vector type. We don't currently have any StrictFP operations that will exercise this code, but I imagine we will when the converts are added.

cameron.mcinally marked an inline comment as done.Jul 19 2018, 12:03 PM

Ping.

This revision is now accepted and ready to land.Jul 22 2018, 2:35 PM
This revision was automatically updated to reflect the committed changes.