This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Disable VLD4 under MVE
ClosedPublic

Authored by dmgreen on Dec 6 2019, 3:55 AM.

Details

Summary

Alas, using half the available vector registers in a single instruction is just too much for the register allocator to handle. The mve-vldst4.ll test here fails when they are enabled at present. Whilst I look into ways to fix that, this disables the generation of VLD4 by adding a mve-max-interleave-factor option, which we currently default to 2.

Diff Detail

Event Timeline

dmgreen created this revision.Dec 6 2019, 3:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 6 2019, 3:55 AM
SjoerdMeijer added inline comments.Dec 6 2019, 4:08 AM
llvm/lib/Target/ARM/ARMISelLowering.cpp
147

Nit: not sure we need to say "Defaults" to 2 as I don't see much prior art of mentioning the default value (it would also require to keep the message up to date with the value when that gets changed, although that is of course not a really difficult problem).

llvm/test/Transforms/LoopVectorize/ARM/mve-vldn.ll
5

Perhaps silly question, but what happens when we pass -mve-max-interleave-factor=42, i.e. a value > 4? Is input checked? Do we need to mention that acceptable values are 1, 2, and 4?

dmgreen marked 2 inline comments as done.Dec 6 2019, 7:53 AM
dmgreen added inline comments.
llvm/lib/Target/ARM/ARMISelLowering.cpp
147

I thought I had seen this in more places, but you are probably right, I'll take that bit out.

llvm/test/Transforms/LoopVectorize/ARM/mve-vldn.ll
5

I was going for not caring about the internal compiler options being given strange values. I think it would likely hit an assert if you put it too high. Hopefully something we don't need to worry about in any case?

The nice thing about an option like this is that we can also set it to 1 if we need to disable the vldn's entirely.

SjoerdMeijer added inline comments.Dec 6 2019, 8:04 AM
llvm/test/Transforms/LoopVectorize/ARM/mve-vldn.ll
5

Agreed.

dmgreen updated this revision to Diff 232570.Dec 6 2019, 8:14 AM
SjoerdMeijer accepted this revision.Dec 6 2019, 8:22 AM

Currently this prevents us from compiling quite a few codes, so this LGTM while we sort things out.

This revision is now accepted and ready to land.Dec 6 2019, 8:22 AM
This revision was automatically updated to reflect the committed changes.