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.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Do you understand why all those test outputs have changed as a side effect of this?
llvm/unittests/Target/ARM/MachineInstrTest.cpp | ||
---|---|---|
939–946 | 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. | |
975 | Another instance of the same typo. |
Are we talking about CONSTRAINED_UNPREDICTABLE here? If so, why is this modeled with hasSideEffects?
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.
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.
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 | ||
---|---|---|
975 | Ah thanks. I've been doing that a lot lately. |
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.