Refactor the scheduling predicates based on MCInstPredicate. In this case, for the Exynos processors.
Thanks for the patch.
Could you please update the diff?
I tried to apply your patch, but there were failures.
AArch64InstrInfo::isExynosShiftFast() is referenced by your patch, but it doesn't appear to exist upstream. In another case, the conflict was caused by a diff which appears to have been committed already.
|130–133 ↗||(On Diff #176891)|
This change was already committed at r348398.
|708 ↗||(On Diff #176891)|
There is no isExynosShiftFast().
|253–255 ↗||(On Diff #176891)|
Not sure where isExynosShiftFast declaration comes from. It is not available at HEAD revision. So, this patch fails to apply.
Unfortunally, I am still seeing several tablegen errors.
Tablegen complains about missing definitions. It looks like most (if not all) of those missing definitions will be added by D55375.
I reported only a few missing symbols. Is this patch meant to be dependent on D55375? If not, then this patch should be fixed. If it is going to be complicated, then I don't mind if you merge the two patches into a single diff. As long as I can build it, and check that the generated code is still semantically equivalent to what there was before, then I am happy.
Although this patch is supposed to be an NFC, I'd like to see at least one llvm-mca test. Ideally, a test that shows how we can now analyze instructions that llvm-mca was previously unable to analyze.
|940–941 ↗||(On Diff #176987)|
For the record: this method never existed in the original code. Not sure how it ended up in this diff.
|25 ↗||(On Diff #176987)|
IsArithExt32Op is not defined.
|27 ↗||(On Diff #176987)|
CheckExtBy0 is not defined.
|35 ↗||(On Diff #176987)|
IsArithExt64Op is not defined.
|48 ↗||(On Diff #176987)|
These two definitions are missing. I saw you added those in D55375.
I went through the various changes, and I didn’t find anything that looks obviously wrong. The new predicates look fine to me.
For the tests, I recommend you to use flag -instruction-table. As an example, see the RUN line in test/tools/llvm-mca/X86/BtVer2/resources-avx1.s.
Flag -instruction-tables enables printing the theoretical resource distribution. In my experience, that’s the best way to check the resource consumption of every instruction.
That flag, plus the implicit instruction-info view, is the best combination for tests where you want to verify latency/throughput and resource usage.
It is particularly useful in the presence of instructions that require resolving write variants; you can basically check all the interesting variants.
As soon as tests are added, and you are happy with the latency/throughput/resource usage of instructions, I will be happy to accept this patch.