This is an archive of the discontinued LLVM Phabricator instance.

GlobalISel: Fix lowerSelect handling of boolean high bits
ClosedPublic

Authored by arsenm on Apr 11 2022, 7:21 PM.

Details

Summary

This was making several invalid assumptions about the incoming
select. First, it was assuming the incoming condition was either s1 or
already sign extended, not accounting for different boolean high bits
behavior between scalar and vector conditions. We only had a vector
boolean due to the intermediate step vector select, which is now
avoided.

Second, it was assuming it can use the result vector type as a boolean
mask. These types don't have anything to do with other, and only makes
sense in the context of the expansion to bit operations. Since these
logically are part of the same lowering, do the complete expansion in
a single step.

The added select_v4s1_s1 test does fail to legalize, since it seems
AArch64's vector legalization support is pretty incomplete.

Diff Detail

Event Timeline

arsenm created this revision.Apr 11 2022, 7:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 11 2022, 7:21 PM
arsenm requested review of this revision.Apr 11 2022, 7:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 11 2022, 7:21 PM
arsenm retitled this revision from GlobalISel: Use correct bool extension for select mask to GlobalISel: Use correct bool extension for select mask.Apr 11 2022, 7:21 PM
arsenm edited the summary of this revision. (Show Details)
arsenm added inline comments.Apr 11 2022, 7:49 PM
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
7278–7279

Actually this whole promotion is wrong. There's no meaningful relationship between the bitwidth of the result type and the compare type. This also doesn't account for the original scalar being promoted

arsenm planned changes to this revision.Apr 12 2022, 7:21 AM

This transform only makes sense to perform if the following bitwise expansion is done at the same time. I'm going to change this to do this legalize in one step

arsenm updated this revision to Diff 422252.Apr 12 2022, 8:47 AM
arsenm retitled this revision from GlobalISel: Use correct bool extension for select mask to GlobalISel: Fix lowerSelect handling of boolean high bits.
arsenm edited the summary of this revision. (Show Details)

Don't use broken intermediate vselect and ensure the boolean is appropriately extended

This revision is now accepted and ready to land.Apr 12 2022, 10:14 AM