Page MenuHomePhabricator

[CodeGen] Add scalable vector support for lowering of llvm.get.active.lane.mask
ClosedPublic

Authored by david-arm on Wed, Nov 24, 8:19 AM.

Details

Summary

Currently the generic lowering of llvm.get.active.lane.mask is done
in SelectionDAGBuilder::visitIntrinsicCall and currently assumes
only fixed-width vectors are used. This patch changes the code to be
more generic and support scalable vectors too. I have added tests
for SVE here:

CodeGen/AArch64/active_lane_mask.ll

although the code quality leaves a lot to be desired. The code will
be improved significantly in a later patch that makes use of the
SVE whilelo instruction.

Diff Detail

Event Timeline

david-arm created this revision.Wed, Nov 24, 8:19 AM
david-arm requested review of this revision.Wed, Nov 24, 8:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Nov 24, 8:19 AM
SjoerdMeijer accepted this revision.Wed, Nov 24, 9:18 AM

although the code quality leaves a lot to be desired.

We never want or actually should fallback to this code, it's just a safety net, and thus I don't think we need to worry at all about code quality. Also, this is just a straightforward expansion how get.active.lane.mask is semantically defined in the LLVM langref; thus I am not even sure there's a more optimal sequence (which again I think is irrelevant). At the moment, this intrinsic is emitted by the LV when MVE is targeted, and when it is emitted, we should always lower it to the MVE target specific intrinsic variant. I guess you're going to do the same for SVE. That's the reason why should never reach this expansion here. But this lowering was added here as a safety net in case we for example one day end up with a user facing version of get.active.lane.mask which a target can't or don't know how to lower.

Long story short, LGTM

This revision is now accepted and ready to land.Wed, Nov 24, 9:18 AM

Hi @SjoerdMeijer, yeah it's really just a safety net, although for SVE there are times when we can't lower this directly to the 'whilelo' instruction. For example, the instruction doesn't support illegal types such as nxv32i1, or inputs that are i8, etc. If we do find ourselves hitting these cases at some point we can look at improving them I guess?

Hi @SjoerdMeijer, yeah it's really just a safety net, although for SVE there are times when we can't lower this directly to the 'whilelo' instruction. For example, the instruction doesn't support illegal types such as nxv32i1, or inputs that are i8, etc. If we do find ourselves hitting these cases at some point we can look at improving them I guess?

Yeah, okay, sure!

Matt added a subscriber: Matt.Thu, Nov 25, 8:51 AM