This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Handle vector UADDO, SADDO, USUBO, SSUBO
ClosedPublic

Authored by nikic on Feb 2 2019, 11:12 AM.

Details

Summary

This is part of https://bugs.llvm.org/show_bug.cgi?id=40442.

It implements vector legalization for the add/sub overflow opcodes. UMULO/SMULO are handled as far as legalization is concerned, but they don't support vector expansion (so no tests for them).

The vector result widening implementation is suboptimal, because it could result in a legalization loop.

Diff Detail

Repository
rL LLVM

Event Timeline

nikic created this revision.Feb 2 2019, 11:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 2 2019, 11:12 AM
arsenm added a subscriber: arsenm.Feb 2 2019, 11:43 AM

I also did this in D57548, so could you copy the test changes from there?

nikic updated this revision to Diff 184909.Feb 2 2019, 12:05 PM
nikic edited the summary of this revision. (Show Details)

Improve vector result widening (now it's general, but could theoretically result in a widen/split loop) and add an AArch64 test (which widens v1i32 as an interesting case).

nikic updated this revision to Diff 184940.Feb 3 2019, 3:42 AM

Short-circuit result assignment if possible. Add AMDGPU tests from D57548.

nikic added a comment.Feb 3 2019, 3:47 AM

@arsenm Sorry, I wasn't aware that someone is already working on this :( I've added the AMDGPU tests now.

arsenm added inline comments.Feb 4 2019, 2:52 PM
lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
260–266 ↗(On Diff #184940)

You can use DAG.ExtractVectorELements here

nikic marked an inline comment as done.Feb 5 2019, 11:17 AM
nikic added inline comments.
lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
260–266 ↗(On Diff #184940)

Something like this?

SmallVector<SDValue, 1> ElemsLHS, ElemsRHS;
DAG.ExtractVectorElements(N->getOperand(0), ElemsLHS);
DAG.ExtractVectorElements(N->getOperand(1), ElemsRHS);
ScalarLHS = ElemsLHS[0];
ScalarRHS = ElemsRHS[0];

It seems a bit odd to go through a vector if only one element is extracted.

nikic updated this revision to Diff 185805.Feb 7 2019, 10:03 AM

Use DAG.ExtractVectorELements().

RKSimon added inline comments.Feb 7 2019, 10:51 AM
test/CodeGen/X86/vec_saddo.ll
1748 ↗(On Diff #185805)

Please can you use nounwind on tests to avoid cfi getting in the way (I tend to add it to all basic codegen tests like these but its up to you):
e.g.

define <2 x i32> @saddo_v2i128(<2 x i128> %a0, <2 x i128> %a1, <2 x i128>* %p2) nounwind {
nikic updated this revision to Diff 185821.Feb 7 2019, 10:58 AM

Add nounwind to avoid cfi.

@arsenm anything else to add?

arsenm accepted this revision.Feb 7 2019, 11:21 AM

LGTM

This revision is now accepted and ready to land.Feb 7 2019, 11:21 AM
RKSimon accepted this revision.Feb 7 2019, 11:25 AM

LGTM too

This revision was automatically updated to reflect the committed changes.