This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Make remaining MVE instruction predictable
ClosedPublic

Authored by dmgreen on Mar 27 2020, 2:09 AM.

Details

Summary

The unpredictable/hasSideEffects flag is usually inferred by tablegen from whether the instruction has a tablegen pattern (and that pattern only has a single output instruction). Now that the MVE intrinsics are all committed and producing code, the remaining instructions still marked as unpredictable need to be specially handled. This adds the flag directly to instructions that need it, notably the V*MLAL instructions and some of the MOV's.

Diff Detail

Event Timeline

dmgreen created this revision.Mar 27 2020, 2:09 AM

Do you understand why all those test outputs have changed as a side effect of this?

llvm/unittests/Target/ARM/MachineInstrTest.cpp
831–838

Typo: "unpredicatable" and "unpredictable" are not the same word, and in MVE, it is actually confusing to use the wrong one of them!

The same typo is in the headline of the commit message.

867

Another instance of the same typo.

Are we talking about CONSTRAINED_UNPREDICTABLE here? If so, why is this modeled with hasSideEffects?

Are we talking about CONSTRAINED_UNPREDICTABLE here?

No, we're talking about MCInstrDesc::hasUnmodeledSideEffects(): not unpredictable in the sense that the architecture doesn't say what the instruction does, but just in the sense that the compiler doesn't know.

dmgreen marked an inline comment as done.Mar 27 2020, 4:27 AM

Are we talking about CONSTRAINED_UNPREDICTABLE here? If so, why is this modeled with hasSideEffects?

No, different thing. It is if the instruction might do things we don't model directly. Read memory, operate on unmodelled flags, those kind of things. They essentially create scheduling barriers, which is why it's good to remove them.

Do you understand why all those test outputs have changed as a side effect of this?

The VMOV lane change I believe. They come from COPY's, not from being directly lowered. Essentially different scheduling graph -> different ordering. I don't think the tests in question are particularly interesting, just showing expansion of VLDn's that we don't have direct support for. With the schedules we have downstream this seems to help in a few cases (so long as D76909 is fixed).

llvm/unittests/Target/ARM/MachineInstrTest.cpp
867

Ah thanks. I've been doing that a lot lately.

dmgreen updated this revision to Diff 253082.Mar 27 2020, 4:32 AM
dmgreen retitled this revision from [ARM] Make remaining MVE instruction predicatable to [ARM] Make remaining MVE instruction predictable.

Unpredicatable -> Unpredictable

simon_tatham accepted this revision.Mar 27 2020, 5:08 AM
This revision is now accepted and ready to land.Mar 27 2020, 5:08 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 2 2020, 3:14 AM