Page MenuHomePhabricator

[AIX] Allow safe for 32bit P8 VSX pattern matching
ClosedPublic

Authored by ZarkoCA on Mar 3 2021, 8:19 PM.

Details

Summary

Pull some of the safe for 32bit pattern matching for Pwr8 and above.

Diff Detail

Event Timeline

ZarkoCA created this revision.Mar 3 2021, 8:19 PM
ZarkoCA requested review of this revision.Mar 3 2021, 8:19 PM
sfertile added inline comments.Mar 5 2021, 12:24 PM
llvm/lib/Target/PowerPC/PPCInstrVSX.td
3185–3186

This pattern should be moved to an IsPPC64 block.

llvm/test/CodeGen/PowerPC/aix-p8-scalar_vector_conversions.ll
2 ↗(On Diff #328007)

I think we should have the ppc64 test in p8-scalar_vector_conversions.ll since it is essentially the same as the BE results in there but without the descriptive register names. Then rename this file staring with aix32.

ZarkoCA updated this revision to Diff 329486.Mar 9 2021, 3:23 PM

removed an unsafe for 32bit pattern match
reworked test cases per comments

ZarkoCA marked an inline comment as done.Mar 9 2021, 3:26 PM

I think we should have the ppc64 test in p8-scalar_vector_conversions.ll since it is essentially the same as the BE results in there but without the descriptive register names. Then rename this file staring with aix32.

Thanks, that's a good suggestion. I added a line in between the LE/BE/AIX test to hopefully make it easier to read in that file.

ZarkoCA updated this revision to Diff 330336.Mar 12 2021, 12:00 PM

Previous version of the patch only legalized the EXTRACT_VECTOR_ELT node in 64Bit mode. Remove that condition so that we are using VSX instructions to extract vector elements in 32bit mode when we can

nemanjai requested changes to this revision.Fri, Mar 19, 9:17 PM

The change is straightforward, but it kind of needs another review.

llvm/lib/Target/PowerPC/PPCInstrVSX.td
3208

Moving this here is just half the story. You will also have to add a 32-bit version of this along with the respective test. It should be straightforward to implement
VectorExtractions.BE_32B_VARIABLE_FLOAT - just use RLWINM instead of RLDICR.

The test to add would be something like:

define float @test(<4 x float> %a, i32 %idx) local_unnamed_addr #0 {
entry:
  %vecext = extractelement <4 x float> %a, i32 %idx
  ret float %vecext
}
This revision now requires changes to proceed.Fri, Mar 19, 9:17 PM
ZarkoCA updated this revision to Diff 333272.Thu, Mar 25, 5:54 AM
  • Added variable float and double extraction for 32Bit
ZarkoCA added inline comments.Thu, Mar 25, 6:05 AM
llvm/lib/Target/PowerPC/PPCInstrVSX.td
2204

This was giving me trouble and @nemanjai helped. Since the 32bit PPC implementation needs to account for SPE, the definition of GPRC_NOR0 requires we type ZERO.

3208

I added this and a 32bit version of variable double extraction as well.

Thank you for the offline help in addressing the issues in implementing this.

nemanjai accepted this revision.Tue, Apr 13, 4:28 AM

LGTM aside from a couple of cosmetic nits that can be fixed on the commit.

llvm/lib/Target/PowerPC/PPCInstrVSX.td
2191

Nit: spaces after operands.

2207

Nit: please align operands to VPERM here.

This revision is now accepted and ready to land.Tue, Apr 13, 4:28 AM
This revision was automatically updated to reflect the committed changes.