[PATCH] D47943: Sample code for porting MachinePipeliner to AArch64+SVE

[Masaki Arai via Phabricator via llvm-commits]

masakiarai added a comment.

Thank you for your detailed comment.
Your points make sense to me.

I should have clarified the intent of this patch.
I do not think that this patch will be accepted.
This is aimed to show that we can generate code without DFAPacketizer with as few changes as possible.
For that reason, the method interface and code keeps the original code as much as possible.



================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:5106
+unsigned
+AArch64InstrInfo::reduceLoopCount(MachineBasicBlock &MBB, MachineInstr *IndVar,
+                                  MachineInstr &Cmp,
----------------
fhahn wrote:
> Does this implementation satisfy the interface? According to TargetInstrInfo::reduceLoopCount, it should generate code to reduce the loop iteration by one.
Hexagon uses special loop instructions to count down the loop counter.
On the other hand, in most cases on AArch64 or x86_64, we will target loops that count up the loop counter.
Therefore, I think that it is appropriate to make the function name `TargetInstrInfo::fixLoopCount' rather than `TargetInstrInfo::reduceLoopCount'．
I thought it was inappropriate to rewrite code for Hexagon, so I did not change it.



================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:5114
+    if (I->getOpcode() == AArch64::SUBSXrr) {
+      CompMI = &*I;
+    }
----------------
fhahn wrote:
> Couldn't we stop after we found the SUBSXrr closest to the terminator?
Yes, you are right.
I should break the loop when I found the first CompMI.
There is the same bug for X86InstrInfo::reduceLoopCount.



https://reviews.llvm.org/D47943