Page MenuHomePhabricator

[AArch64][sve] Prevent incorrect function call on fixed width vector
ClosedPublic

Authored by DavidTruby on Sep 2 2021, 9:05 AM.

Details

Summary

The isEssentiallyExtractHighSubvector function currently calls
getVectorNumElements on a type that in specific cases might be scalable.
Since this function only has correct behaviour at the moment on scalable
types anyway, the function can just return false when given a fixed type.

Diff Detail

Event Timeline

DavidTruby created this revision.Sep 2 2021, 9:05 AM
DavidTruby requested review of this revision.Sep 2 2021, 9:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 2 2021, 9:05 AM
Matt added a subscriber: Matt.Sep 2 2021, 12:04 PM
bsmith added inline comments.Sep 3 2021, 5:36 AM
llvm/test/CodeGen/AArch64/sve-no-typesize-warnings.ll
35

This test is missing a vscale_range attribute that would have triggered the assertion failure

paulwalker-arm added inline comments.Sep 3 2021, 8:28 AM
llvm/test/CodeGen/AArch64/sve-no-typesize-warnings.ll
35

The test also looks more complicated than need be. I've had a play and came up with:

; RUN: llc < %s | FileCheck %s

target triple = "aarch64-unknown-linux-gnu"

define <4 x i32> @sve_no_typesize_warning(<vscale x 8 x i16> %a, <4 x i16> %b) #0 {
  %a.lo = call <4 x i16> @llvm.experimental.vector.extract.v4i16.nxv8i16(<vscale x 8 x i16> %a, i64 0)
  %a.lo.zext = zext <4 x i16> %a.lo to <4 x i32>
  %b.zext = zext <4 x i16> %b to <4 x i32>
  %add = add <4 x i32> %a.lo.zext, %b.zext
  ret <4 x i32> %add
}

declare <4 x i16> @llvm.experimental.vector.extract.v4i16.nxv8i16(<vscale x 8 x i16>, i64)

attributes #0 = { "target-features"="+sve" }

Which looks to do the job. The test is using only scalable and NEON fixed length types so there's no need for vscale_range.

DavidTruby updated this revision to Diff 370893.Sep 6 2021, 4:53 AM

Change test as suggested by @paulwalker-arm

llvm/test/CodeGen/AArch64/sve-no-typesize-warnings.ll
35

Looks good to me, I've just changed the test to be this instead. Thanks!

DavidTruby marked 2 inline comments as done.Sep 6 2021, 4:53 AM
peterwaller-arm accepted this revision.Sep 6 2021, 5:09 AM
This revision is now accepted and ready to land.Sep 6 2021, 5:09 AM
paulwalker-arm accepted this revision.Sep 6 2021, 5:48 AM
This revision was landed with ongoing or failed builds.Sep 6 2021, 6:25 AM
This revision was automatically updated to reflect the committed changes.