This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE] Fix bad PTEST(PG, OP(PG, ...)) optimization
ClosedPublic

Authored by c-rhodes on Nov 9 2022, 7:42 AM.

Details

Summary

AArch64InstrInfo::optimizePTestInstr attempts to remove a PTEST of a
predicate generating operation that identically sets flags (implictly).

When the PTEST and the predicate-generating operation use the same mask
the PTEST is currently removed. This is incorrect since it doesn't
consider element size. PTEST operates on 8-bit predicates, but for
instructions like compare that also support 16/32/64-bit predicates, the
implicit PTEST performed by the instruction will consider fewer lanes
for these element sizes and could set different first or last active
flags.

For example, consider the following instruction sequence

  ptrue p0.b			; P0=1111-1111-1111-1111
  index z0.s, #0, #1		; Z0=<0,1,2,3>
  index z1.s, #1, #1		; Z1=<1,2,3,4>
  cmphi p1.s, p0/z, z1.s, z0.s  ; P1=0001-0001-0001-0001
				;       ^ last active
  ptest p0, p1.b		; P1=0001-0001-0001-0001
				;     ^ last active

where the compare generates a canonical all active 32-bit predicate (equivalent
to 'ptrue p1.s, all'). The implicit PTEST sets the last active flag, whereas
the PTEST instruction with the same mask doesn't.

This patch restricts the optimization to instructions operating on 8-bit
predicates. One caveat is the optimization is safe regardless of element
size for any active, this will be addressed in a later patch.

Diff Detail

Event Timeline

c-rhodes created this revision.Nov 9 2022, 7:42 AM
c-rhodes requested review of this revision.Nov 9 2022, 7:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 9 2022, 7:42 AM
Matt added a subscriber: Matt.Nov 11 2022, 7:28 PM
bsmith accepted this revision.Nov 14 2022, 2:59 AM
bsmith added a subscriber: bsmith.

LGTM, modulo future changes

This revision is now accepted and ready to land.Nov 14 2022, 2:59 AM
This revision was landed with ongoing or failed builds.Nov 15 2022, 2:54 AM
This revision was automatically updated to reflect the committed changes.