This is an archive of the discontinued LLVM Phabricator instance.

[LegalizeTypes][VP] Add splitting support for vp.gather and vp.scatter
ClosedPublic

Authored by victor-eds on Jan 20 2022, 2:08 AM.

Details

Summary

Split these nodes in a similar way as their masked versions.

Diff Detail

Event Timeline

victor-eds created this revision.Jan 20 2022, 2:08 AM
victor-eds requested review of this revision.Jan 20 2022, 2:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 20 2022, 2:08 AM
frasercrmck added inline comments.Jan 20 2022, 4:10 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
2037

Is it possible to test this case?

2321

I think we should also have to split VP_GATHER operands, much like MGATHER here.

2973

And test this, if possible.

victor-eds edited the summary of this revision. (Show Details)

Share code

victor-eds marked an inline comment as done.

Rename variable

Add SplitVecOp_Gather tests for VP_GATHER

Use SplitMask

craig.topper added inline comments.Jan 21 2022, 10:16 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
1948

Use and else and a cast instead of the llvm_unreachable?

2006

Just use an else with a cast inside and drop the llvm_unreachable.

Simplify code

Apply suggested changes

victor-eds marked 2 inline comments as done.Jan 21 2022, 11:11 AM

Done

victor-eds added inline comments.Jan 21 2022, 11:14 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
2037

I think it wasn't, but it doesn't apply anymore.

2973

Not sure if this applies anymore.

craig.topper added inline comments.Jan 22 2022, 5:05 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
1957–1958

I'm not sure if this is important or not, but may be a change in behavior here. If this function gets called from SplitVecOp_Gather, we'll check the opcode and maybe call SplitVecRes_SETCC for the mask. Previously SplitVecOp_MGATHER wouldn't do anything special for the mask.

I notice that SplitVecOp_MSCATTER only called it when OpNo==1 and not when legalizing the index or the mask.

Maybe we should have a bool parameter to SplitVecRec_Gather to distinquish the caller to not change this behavior.

Add boolean parameter to split SetCC operand or not

victor-eds marked an inline comment as done.Jan 24 2022, 1:38 AM

Missed that. Thanks. This should handle this behavior change.

LGTM, I think. Craig's left more detailed feedback so I'll defer to him.

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
948

/*SplitSETCC*/ true

1940

nit: Operands rather than Operators, I think.

Apply suggested changes

This revision is now accepted and ready to land.Jan 24 2022, 8:57 AM
frasercrmck accepted this revision.Jan 25 2022, 1:55 AM

LGTM too. Minor typo in the description: they're -> their

victor-eds edited the summary of this revision. (Show Details)Jan 25 2022, 2:05 AM