This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Expand vXi1 VSELECT's
ClosedPublic

Authored by dmgreen on Jan 18 2021, 10:02 PM.

Details

Summary

We have no lowering for VSELECT vXi1, vXi1, vXi1, so mark them as expanded to turn them into a series of logical operations.

Diff Detail

Event Timeline

dmgreen created this revision.Jan 18 2021, 10:02 PM
dmgreen requested review of this revision.Jan 18 2021, 10:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 18 2021, 10:02 PM
SjoerdMeijer added inline comments.Jan 18 2021, 11:37 PM
llvm/test/Analysis/CostModel/ARM/arith-overflow.ll
387

More of a curiosity, how was this lowered before? Given the added codegen tests, I guess this is not only a cost-model tweak? And some of these costs are extremely high. Do they make sense?

dmgreen added inline comments.Jan 19 2021, 2:52 AM
llvm/test/Analysis/CostModel/ARM/arith-overflow.ll
387

Yep. The costs are based on whether the operation is legal and add.with.overflow checks the cost of selects of predicates (v4i1). There is no legal instruction for them for MVE, so a high cost is probably fine.

D94958 updates the similar costs for sadd.sat and friends, which also have costs based in sadd.with.overflow by default.

SjoerdMeijer accepted this revision.Jan 19 2021, 5:37 AM
SjoerdMeijer added inline comments.
llvm/test/Analysis/CostModel/ARM/arith-overflow.ll
387

Yep. The costs are based on whether the operation is legal and add.with.overflow checks the cost of selects of predicates (v4i1). There is no legal instruction for them for MVE, so a high cost is probably fine.

D94958 updates the similar costs for sadd.sat and friends, which also have costs based in sadd.with.overflow by default.

Yeah, thanks, now this is more consistent with each other; I was asking because of this difference.

LGTM

This revision is now accepted and ready to land.Jan 19 2021, 5:37 AM
This revision was automatically updated to reflect the committed changes.